Integration property tests#122
Conversation
|
Thanks for taking this! This is a non trivial change so I'll need a day or two to review this (I'm also traveling atm). Will get back soon! |
|
|
||
| proptest! { | ||
| #[test] | ||
| fn proptest_trapezoid_integration_f64( |
There was a problem hiding this comment.
You added four tests, but there is so much repeated code. Please consolidate them into one test.
There was a problem hiding this comment.
@kmolan Yeah, you're right. I could factor out a lot of the code into a function returning a strategy which includes the function, the closed form value, the tolerance and the limit. But do you really want all the integration methods to live in a single test? Should they not be allowed to succeed/fail individually?
There was a problem hiding this comment.
no, this is cleanup and therefore nice to have, not a strong blocker.
|
@kirloo Left comments. Please also add tests for other quadratures when ready. |
3 participants
What & why
Resolves #102
I haven't touched
gaussian_tablesyet, but I'm submitting a draft because while the test logic looks sound, I had to increase the number of iterations all the way to 300 for the Simpsons test to pass. I don't know whether that's indicative of an error somewhere, or expected for that method due to the relatively larger intervals, but the discrepancy made me nervous.Checklist
## [Unreleased](required for behavior-facing changes)cargo test+cargo clippy --all-targetsclean locallyunwrap/expect/panicon library paths (typed errors instead)