Reliable release - #25
Merged
Merged
Conversation
Five tests failed sporadically, together on roughly one run in four. All of them share one cause: they assert on unseeded randomness. The augmentations draw from the global numpy stream, the mock Sintel data is generated fresh every run, and several assertions only hold for most draws rather than all of them. In CI a sporadic failure is indistinguishable from a real regression. Seed the global generators before every test, and seed the session-scoped mock data fixtures with their own generator, since those are built before the per-test fixture runs. FLYVIS_TEST_SEED re-runs the suite under a different seed; an assertion that only holds for the default seed is a broken assertion, and sweeping the seed is how to find one. Seeding alone only makes a wrong assertion fail reproducibly, so fix the five assertions that were true by luck: - test_sintel::test_getitem (8.1%) and test_sintel::test_apply_augmentation (10%) asserted that augmentation changes the target. Targets are only rotated, flipped and cropped -- they are neither jittered nor noised -- so a sampled augmentation leaves them untouched whenever it draws neither a rotation nor a flip: (1-p_rot)*(1-p_flip) = 1/12. Request a rotation explicitly instead of asserting on the draw. - test_ensemble::test_rank_by_validation_error (4.2%) shuffled the model names and asserted the order changed. A shuffle reproduces the sorted order once in len(ensemble)! draws, 1 in 24 here. Rotate instead, which is a derangement. - test_rendering::test_call (2.3%) compared the mean of a median-filtered uniform sample against 5 with atol=0.05, about 2.4 sigma of that statistic. Widen to ~7 sigma and say why; a seeded draw also differs between CPU and GPU, so a tight tolerance would pass locally and fail in CI. - test_augmentation::test_random_crop (~0.1%) asserted a random crop differs from the crop at frame 0, which fails when the sampled start is 0. Assert that the sampled start is the one applied, which is exact and stronger. Found and verified by sweeping the seed rather than repeating runs: repeating the suite 15 times missed two of these, while sweeping 40 seeds found both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Releasing is fully manual today: build locally, then twine upload. Automate it so that a release is a tag plus a GitHub Release and nothing else. Every push to main builds the sdist and the wheel and checks them, so a packaging mistake surfaces at merge time. Nothing is uploaded on a merge -- PyPI versions are immutable and cannot be reused, so publishing is tied to a version tag rather than to every commit that lands on main. Before uploading, the workflow refuses to continue if the version derived by setuptools_scm does not match the release tag, if twine check fails, or if flyvis/data/responses_norm.h5 is missing from either distribution. That last check matters because the constants are loaded from inside the installed package: a wheel without them would silently fall back to simulating 30 minutes of naturalistic stimuli per model. It then installs the wheel in a clean virtualenv and reads the constants back before publishing. Uploads use PyPI trusted publishing, so no API token is stored in the repository. The one-time PyPI-side setup is documented in docs/release.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
=======================================
Coverage 38.68% 38.68%
=======================================
Files 75 75
Lines 9738 9738
=======================================
Hits 3767 3767
Misses 5971 5971
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.