Skip to content

Reorganize inference/ into one pilot_inference package - #246

Open
jmacd867 wants to merge 4 commits into
mainfrom
Organization
Open

jmacd867 wants to merge 4 commits into
mainfrom
Organization

Conversation

@jmacd867

Copy link
Copy Markdown
Collaborator

Closes #244

Summary

inference/ becomes one installed package, grouped by the concerns in GLOSSARY.md. Event classification, which the client never asked for, stays in its own folder and is marked out of scope.

 inference/
-├── angles.py, features.py, clustering.py, ...   # 12 scripts and modules, flat
-├── Data/  Models/  Results/  dataset/           # mixed case, code inside Data/
-├── requirements.txt
+├── data/  models/                               # lowercase, no code
+│   └── prepared/                                # was dataset/
+├── pilot_inference/
+│   ├── angles.py, paths.py                      # shared
+│   ├── capture/                                 # data_logger
+│   ├── performance/                             # features, clustering, synthetic pilots
+│   └── event_classification/                    # OUT OF SCOPE; test_model.py is now evaluate_model.py
 └── tests/                                       # mirrors pilot_inference/

Run scripts as uv run python -m pilot_inference.<group>.<module>, from inference/ or from the repo root with --project inference. pyproject.toml gains a build-system, so uv sync installs the package editable.

Also: GLOSSARY.md gains Segment, Instruction and Leg, and the event label terms move to an out-of-scope section. The root README.md, inference/README.md, AGENTS.md, docs/telemetry_schema.md and ADR 0002 are updated for the new paths and commands. The .pkl models stop being tracked (they were already in .gitignore), and train_model writes only bestModel.pkl, not an identical finalModel.pkl. requirements.txt and two empty files are removed.

Evidence

  • Before: uv run pytest gave 73 passed. The flat scripts ran as python inference/cluster_pilots.py, and test_data_logger.py needed sys.path.insert(... / "Data") to import the logger.
    After: uv run pytest gives 73 passed (same tests, moved and re-imported). The logger test imports pilot_inference.capture.data_logger.
generate_synthetic_pilots -> compute_features -> cluster_pilots --k 3 --truth ...
before: adjusted Rand index 0.79 (documented in the old README)
after:  adjusted Rand index 0.792, run via python -m from inference/

Every event_classification script also ran end to end via python -m. The data logger needs a live relay, so it was not run. Anything needing X-Plane or iMotions is unverified.

Merge Danger

Door: two-way. It is moves and edits, and a revert restores the old layout.

Two things are not a clean undo for other people. The .pkl models are no longer tracked, so a fresh clone must run train_model before evaluate_model. raw_flight_data_updated.csv was meant to be pruned but is kept: it is the source of the committed labeled_flight_data.csv, and running label_generator as documented overwrites that file with the 597-row raw_flight_data.csv version (documented in inference/README.md).

Blast Radius: inference/

Anyone with a local inference/.venv must run uv sync again. Open branches that touch inference/ will conflict with the moves (PR #176 changes 6 files there). Open issues #212 and #229 cite the old Data/ and Results/ paths. Data/ to data/ is a case-only rename, so on a case-insensitive checkout a stale Data/ folder can linger until it is deleted by hand.

Move the flat scripts into inference/pilot_inference/, grouped by the
domain concerns in GLOSSARY.md:

- capture/: data_logger
- performance/: deviation features, clustering, synthetic pilots
- event_classification/: the rule-based labeler and Random Forest, kept
  apart because the client has not asked for it
- angles.py and paths.py shared at the package root

The package is installed editable (hatchling build-system), so scripts
run as `python -m pilot_inference.<group>.<module>` from any directory.
Paths for data/ and models/ come from paths.py instead of each script's
own __file__.

Also:
- lowercase Data/, Models/ and dataset/ (prepared splits now live in
  data/prepared/); tests/ mirrors the package
- test_model.py -> evaluate_model.py, since it is not a test
- stop tracking models/*.pkl (already in .gitignore) and write only
  bestModel.pkl, not an identical finalModel.pkl
- drop requirements.txt, Results/results.txt and Models/sample_Models.txt
- keep raw_flight_data_updated.csv: it is the source of the committed
  labeled dataset, documented in inference/README.md
- GLOSSARY.md: add Segment and Instruction, move the event
  classification terms to an out-of-scope section
- update README, AGENTS.md, docs and .gitignore for the new paths
- GLOSSARY.md: put the out-of-scope terms under a subheading of Language
  and its relationship under Relationships, drop implementation detail
  from the section intro and the Segment definition, and stop the
  Segment and Flight phase _Avoid_ lists overlapping on "stage"
- evaluate_model.py: describe it as an evaluation, not a test
- error and hint messages now give the python -m form
- name the project pilot-inference to match the package
- README: drop the stale scikit-learn pickle note
- formatting: blank lines after the new imports, wrap one docstring line
- .gitignore: explain why both Client spellings are listed
@jmacd867
jmacd867 requested a balanced review from Copilot October 22, 2026 16:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jmacd867
jmacd867 requested a review from Phlabry October 22, 2026 16:20
@Sanjana-Siddani
Sanjana-Siddani self-requested a review October 24, 2026 17:13

@Sanjana-Siddani Sanjana-Siddani left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big cleanup, nice work! I followed the README from scratch: uv sync, all 73 tests pass, and every command works from inference/ and from the root. Clustering gives the same result as before (0.792), and train → evaluate works on a fresh clone without the .pkl files. One small idea for later: running the event-classification steps still overwrites the committed files in data/prepared/, so maybe stop tracking those at some point. Approving

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reorganize the inference/ folder

3 participants