Skip to content
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

Function for replacing objectivePrior* by observables/measurements #309

Merged
merged 9 commits into from
Oct 15, 2024

Conversation

dweindl
Copy link
Member

@dweindl dweindl commented Sep 24, 2024

Reformulate the given PEtab problem such that the objective priors are converted to measurements. This is done by adding a new observable for each prior and adding a corresponding measurement to the measurement table. The resulting optimization problem will be equivalent to the original problem. This is meant to be used for tools that do not support priors.

Closes #307

dweindl and others added 2 commits September 24, 2024 10:47
Reformulate the given PEtab problem such that the objective priors are converted
to measurements. This is done by adding a new observable for each prior
and adding a corresponding measurement to the measurement table.
The resulting optimization problem will be equivalent to the original problem.
This is meant to be used for tools that do not support priors.

Closes PEtab-dev#307
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 70.90909% with 16 lines in your changes missing coverage. Please review.

Project coverage is 74.95%. Comparing base (fdd9d95) to head (3e9e17b).

Files with missing lines Patch % Lines
petab/v1/priors.py 70.37% 7 Missing and 9 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #309      +/-   ##
===========================================
+ Coverage    72.86%   74.95%   +2.09%     
===========================================
  Files           51       52       +1     
  Lines         4842     4896      +54     
  Branches       826      838      +12     
===========================================
+ Hits          3528     3670     +142     
+ Misses        1018      921      -97     
- Partials       296      305       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dweindl dweindl requested a review from dilpath October 14, 2024 08:21
Copy link
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks great, thanks!

petab/v1/priors.py Outdated Show resolved Hide resolved
petab/v1/priors.py Outdated Show resolved Hide resolved
petab/v1/priors.py Outdated Show resolved Hide resolved
petab/v1/priors.py Outdated Show resolved Hide resolved
petab/v1/priors.py Outdated Show resolved Hide resolved
Comment on lines +98 to +104
prior_contrib += norm.logpdf(
petab_problem_priors.x_nominal_free_scaled[
petab_problem_priors.x_free_ids.index(parameter_id)
],
loc=prior_pars[0],
scale=prior_pars[1],
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks useful, could be moved to some calculate_posterior_for_table method like calculate_llh_for_table

Copy link
Member Author

@dweindl dweindl Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fully agree. However, I would like to have some proper Prior class instead of those current tuples before extending the priors API. I would rather keep that for another PR. I will open an issue.

-> #311, #312

tests/v1/test_priors.py Show resolved Hide resolved
tests/v1/test_priors.py Outdated Show resolved Hide resolved
petab/v1/priors.py Show resolved Hide resolved
@dweindl dweindl marked this pull request as ready for review October 15, 2024 06:41
@dweindl dweindl requested a review from a team as a code owner October 15, 2024 06:41
@dweindl dweindl linked an issue Oct 15, 2024 that may be closed by this pull request
@dweindl dweindl merged commit cf82b88 into PEtab-dev:develop Oct 15, 2024
7 checks passed
@dweindl dweindl deleted the priors_to_measurements branch October 15, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function for replacing objectivePrior* by observables/measurements
3 participants