You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
|**PID.py**| PI controller closed-loop test: CPU target sequence, save dynamics, PNG plots | ✅ run |
8
-
|**feedForward.py**| Open-loop test: sleep time sequence (CPU model), comparison with monitor | ✅ run |
9
-
|**Identification.py**| Identification: for each sleep time estimates mean CPU load, scatter plot sleep vs load | ❌ not in CI |
10
-
|**Identification2.py**| Alternative identification: sleep time sequence, dynamics plot and PNG | ❌ not in CI |
5
+
On every push and pull request to `master`, the [CI workflow](../.github/workflows/ci.yml) runs:
11
6
12
-
---
7
+
-**Python 3.13** on Ubuntu
8
+
- After `pip install -r requirements.txt`:
9
+
-`python tests/PID.py`
10
+
-`python tests/feedForward.py`
13
11
14
-
## Strengths
12
+
If both scripts complete without errors, the build passes. They produce data files and PNGs in the current working directory (repo root when run from CI).
15
13
16
-
-**Component reuse:** Monitor, Controller, ClosedLoopActuator, OpenLoopActuator used consistently.
17
-
-**Reproducibility:**`testing = 1` runs the experiment, `testing = 0` loads data from JSON files (useful to regenerate plots without re-running).
18
-
-**CI:** PID and feedForward are run on push/PR (Python 3.9 and 3.13).
14
+
## Scripts
19
15
20
-
---
16
+
| Script | Run in CI | Description |
17
+
|--------|-----------|-------------|
18
+
|**PID.py**| Yes | Closed-loop (PI) test: runs a sequence of CPU targets, records dynamics, saves `pid_data` and PNGs (e.g. `PID.png`, `PID Actuation.png`). |
19
+
|**feedForward.py**| Yes | Open-loop test: runs a sequence of sleep times, records dynamics, saves `feed_forward_data` and `FeedForward.png`. |
20
+
|**Identification.py**| No | Identification script: sweep sleep times, estimate CPU load, scatter plots. Run manually from project root if needed. |
21
+
|**Identification2.py**| No | Alternative identification: sleep-time sequence and dynamics plot. Run manually from project root if needed. |
-**PID.py** and **feedForward.py** import `_available_cores` from `cpu_load_generator` (same logic as main script) → they work on macOS too.
27
-
28
-
### 2. ~~**Use of `monitor.running` (bug)**~~ (fixed)
29
-
30
-
- In **Identification.py** and **Identification2.py**, `monitor.running = 0` was replaced with **`monitor.stop()`** before `monitor.join()`.
31
-
32
-
### 3. ~~**Duplicate `monitor.join()` in Identification.py**~~ (fixed)
33
-
34
-
- Removed the redundant second `monitor.join()`; a single `monitor.stop()` + `monitor.join()`.
35
-
36
-
### 4. **Path and imports**
37
-
38
-
- All tests use `sys.path.insert(0, ...)` to import from the parent. This works when run from repo root; from `tests/` it may depend on cwd.
39
-
- CI runs from repo root (`python tests/PID.py`), so it is fine. Running from `tests/` might not find `utils`.
40
-
- Optional: use `python -m tests.PID` from root, or document “run from project root”.
41
-
42
-
### 5. **Files and figures written to cwd**
43
-
44
-
- Tests write `pid_data`, `feed_forward_data`, `identification_data`, `scatter_plot_data`, and PNGs (`PID.png`, `FeedForward.png`, `Identification.png`, etc.) to the **current working directory**.
45
-
- In CI the cwd is the repo root → these files end up in the repo and could be committed by mistake (though `.gitignore` has `*.png`).
46
-
-**Suggestion:** write to a subfolder (e.g. `tests/output/` or `tests/artifacts/`) and add it to `.gitignore`, or use `tempfile.gettempdir()`.
47
-
48
-
### 6. **No assertions**
49
-
50
-
- The tests do **not** use `assert` or a framework (pytest/unittest). They only check that the run does not crash and produce data/plots.
51
-
- Useful as characterization/identification scripts; less so as automated tests (e.g. “controller reaches target within X seconds”).
52
-
- Optional: add minimal checks (e.g. `assert len(dynamics['cpu']) > 0`, or thresholds on mean error) to make CI tests more robust.
53
-
54
-
### 7. **Identification and Identification2 not in CI**
55
-
56
-
-**Identification.py** and **Identification2.py** are not run by the CI workflow. If something breaks in Monitor/OpenLoopActuator, CI won’t catch it for these two.
57
-
-**Suggestion:** include them in CI (at least as “run without crash”) or document that they are manual identification scripts.
58
-
59
-
### 8. **Unused variables**
60
-
61
-
-`dynamics_plot_online` is defined but not clearly used to change behaviour (in some places it is only passed to OpenLoopActuator as `plot`). It can be removed or used explicitly.
62
-
63
-
### 9. **Axis labels and titles**
64
-
65
-
- In several places the x-axis is labeled "Time [ms]" while the dynamics use `time.time()` (seconds). It should be "Time [s]" for consistency.
66
-
67
-
---
68
-
69
-
## Recommended actions summary
70
-
71
-
| Priority | Action |
72
-
|----------|--------|
73
-
|~~High~~|~~Make PID.py and feedForward.py macOS-compatible~~ (done: `_available_cores` from `cpu_load_generator`). |
74
-
|~~High~~|~~Replace `monitor.running = 0` with `monitor.stop()`~~ (done in Identification and Identification2). |
75
-
| Medium | Write data and PNGs to `tests/output/` (or similar) and add to `.gitignore`. |
76
-
| Medium | Fix "Time [ms]" → "Time [s]" where data are in seconds. |
77
-
| Low | Add Identification.py and Identification2.py to CI (at least as smoke). |
78
-
| Low | Add some asserts or pytest to turn them into proper automated tests. |
79
-
80
-
---
81
-
82
-
## How to run the tests
23
+
## How to run (same as CI)
83
24
84
25
From the **project root**:
85
26
86
27
```bash
87
28
pip install -r requirements.txt
88
29
python tests/PID.py
89
30
python tests/feedForward.py
90
-
python tests/Identification.py # optional, may open windows
91
-
python tests/Identification2.py
92
31
```
93
32
94
-
The tests are aligned with the main script changes (`cpu_load_generator.py`, cross-platform cores, monitor stop).
33
+
Run from the project root so imports (`utils`, `cpu_load_generator`) resolve correctly.
0 commit comments