-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix test_priors_to_measurements #336
Fix test_priors_to_measurements #336
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #336 +/- ##
========================================
Coverage 74.36% 74.36%
========================================
Files 53 53
Lines 5138 5138
Branches 903 903
========================================
Hits 3821 3821
Misses 977 977
Partials 340 340 ☔ View full report in Codecov by Sentry. |
x_scaled_dict = dict( | ||
zip( | ||
original_problem.x_free_ids, | ||
original_problem.x_nominal_free_scaled, | ||
strict=True, | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it really be scaled values here? I assume the prior is either (1) unscaled or (2) on parameterScale
. The former means this should be unscaled, and the latter is already handled in the observable formulae for these dummy prior observables, so no scaling is required here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far, this test only handles PARAMETER_SCALE_NORMAL (to be extended in #329). Yes, the scaling is handled in the observable formulae, but here, we need to write the simulation table. For that, we do not evaluate the observable formula using the given parameters, but write the result directly. Therefore, we need the scaled ones.
Fixes an issue in `test_priors_to_measurements` which led to evaluating the prior at the wrong parameter values (using the location parameter of the prior instead of the actually estimated parameters). The problem was in the test code, not the tested code.
b15d57e
to
749168c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, then fine as is
Co-authored-by: Dilan Pathirana <[email protected]>
Fixes an issue in
test_priors_to_measurements
which led to evaluating the prior at the wrong parameter values (using the location parameter of the prior instead of the actually estimated parameters). The problem was in the test code, not the tested code.