Repository navigation
Add correctness tests for Euler/RK4 integration - #120
anar-rzayev wants to merge 3 commits into
Conversation
The existing test_euler_integrate / test_rk4_integrate only assert output shapes and dict keys. Add numerical-correctness tests that drive the integrators with analytic velocity fields (via a mocked model) and check against closed-form solutions: - Constant field v = c: the trajectory must be a straight line with total displacement exactly c over t in [0, 1], for both Euler and RK4 and independent of step count. - Decay field v = -x: coarse RK4 (21 steps) must converge to the same solution as fine-step Euler (2000 steps), and be more accurate than Euler at the same step count -- exercising RK4's higher order. Tests use a trivial stub model so they are fast (not marked slow) and are seeded so Euler and RK4 start from identical prior noise. Closes prism-science#58 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DG3ZwCtLWBcmULcWeV5gf7
|
Warning Review limit reachedNext included review available in 53 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR adds FlowMatcher integration tests for Euler and RK4 methods. The tests cover analytical trajectories, RK4 timing and accuracy, reproducible initial noise, time-dependent fields, and batched graph output splitting. ChangesFlow integration validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds numerical and batching correctness tests without altering production behavior or runtime configuration, so no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The tests satisfy issue ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds numerically meaningful integration correctness tests to ensure FlowMatcher.euler_integrate and FlowMatcher.rk4_integrate produce correct trajectories (not just correctly shaped outputs), using lightweight analytic/mock velocity fields to keep runtime low.
Changes:
- Introduces analytic velocity-field mock modules (constant, linear decay, time-ramp, per-graph) to drive integrators deterministically.
- Adds closed-form correctness checks for constant-field trajectories and time-dependent stage timing behavior.
- Adds a convergence test (coarse RK4 vs fine Euler) and a batched multi-graph splitting test to validate per-graph masking/splitting.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/test_flow.py:902
test_rk4_matches_euler_with_more_stepsuses a 2000-step Euler run as the reference. This makes the test slower than necessary and the reference itself is still a numerical approximation even though this ODE has a closed-form solution (x(1)=x(0)*exp(-k)). Using the analytic solution makes the test faster and verifies absolute correctness directly.
seed = 1234
euler_fine = run("euler", num_steps=2000, seed=seed)
rk4_coarse = run("rk4", num_steps=21, seed=seed)
euler_coarse = run("euler", num_steps=21, seed=seed)
# Identical initial noise across the three runs (guards the comparison).
np.testing.assert_allclose(
rk4_coarse["trajectory"][0], euler_fine["trajectory"][0], atol=1e-5
)
np.testing.assert_allclose(
euler_coarse["trajectory"][0], euler_fine["trajectory"][0], atol=1e-5
)
reference = euler_fine["water_pred"]
# Coarse RK4 converges to the fine-step Euler solution.
np.testing.assert_allclose(
rk4_coarse["water_pred"], reference, rtol=2e-2, atol=2e-2
)
# RK4 is closer to the reference than Euler at the same step count.
rk4_err = np.linalg.norm(rk4_coarse["water_pred"] - reference)
euler_err = np.linalg.norm(euler_coarse["water_pred"] - reference)
assert rk4_err < euler_err
test_euler_integrate/test_rk4_integrateonly check shapes and keys. Thesedrive both integrators with analytic velocity fields (model swapped for a mock)
and compare against closed-form solutions:
v = c— straight line, total displacement exactlyc, both methodsv = -x— coarse RK4 matches 2000-step Euler, and beats Euler at equal stepsv = a*t— pins down the times each stage is evaluated atscripts/inference.pypath, previously untested)Tests only;
tests/test_flow.pyis the one file changed.