Phase 3: add eval-cache unit test and CI workflow#1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6b900b366
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name: CI | ||
|
|
||
| on: |
There was a problem hiding this comment.
The block beginning here adds a second complete name/on/jobs mapping before the existing workflow at lines 41–85. GitHub Actions rejects workflow YAML containing duplicate top-level keys, so this file will not load and no CI checks will run for pushes or pull requests until the definitions are merged into a single mapping.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR advances the Phase 3 remediation by adding coverage for _eval_model caching behavior, tightening determinism checks in partitioners, and introducing/adjusting CI and platform-specific execution behavior (notably for Windows Ray/Torch issues).
Changes:
- Added a new unit test to validate
TVFLIDSStrategy._eval_modelevaluation caching. - Expanded partitioner tests to assert deterministic output given a fixed seed; adjusted determinism integration test scaffolding for Windows.
- Added/updated GitHub Actions CI workflow content and introduced a small import-check helper script; minor runtime adjustment to use Ray local mode on Windows.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_eval_cache.py |
Adds a targeted unit test to ensure _eval_model caching is effective. |
tests/test_all.py |
Adds deterministic partitioner tests and wraps determinism integration tests with Windows-specific skipping logic. |
scripts/check_imports.py |
Adds a simple environment diagnostic script for Torch/Flower/Ray imports. |
fl/baselines/fltrust_strategy.py |
Removes an unused import. |
experiments/run_experiment.py |
Initializes Ray in local_mode on Windows to reduce DLL/spawn issues; passes ray_init_args into Flower simulation. |
.github/workflows/ci.yml |
Adds CI workflow steps for Python deps and test execution (but currently duplicates workflow definitions in one file). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class TestDeterminism(unittest.TestCase): | ||
| """Verify that two identical-seed runs produce identical outputs.""" | ||
|
|
||
| def _run_mini(self, seed): | ||
| """Run 1 round of FedAvg and return the final trust-scorer state.""" |
| name: CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: [ main ] | ||
| pull_request: | ||
| branches: [ main ] |
| python -m pip install --upgrade pip | ||
| # Install CPU builds of torch to avoid CUDA dependency on CI | ||
| python -m pip install torch==2.1.0+cpu torchvision==0.16.0+cpu -f https://download.pytorch.org/whl/cpu/torch_stable.html | ||
| python -m pip install -r requirements.txt |
| run: | | ||
| pytest -q |
| @@ -0,0 +1,19 @@ | |||
| import sys | |||
This PR implements Phase 3 remediation changes:
tests/test_eval_cache.pyverifying_eval_modelcaching.github/workflows/ci.ymlto run tests on Ubuntufl/strategy.pyandtrust/verification.pyPlease run CI to validate full integration tests (Windows Ray/Torch issues may block local validation).