Implement proper truncation for prior distributions

[dweindl]

Currently, when sampled startpoints are outside the bounds, their value is set to the upper/lower bounds. This may put too much probability mass on the bounds.

With these changes, we properly sample from the respective truncated distributions.

Closes #330.

This also evaluates all priors on the model parameter scale (instead of parameterScale scale, see PEtab-dev/PEtab#402.

👀 https://petab--335.org.readthedocs.build/projects/libpetab-python/en/335/example/distributions.html

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 82.96296% with 23 lines in your changes missing coverage. Please review.

Project coverage is 74.08%. Comparing base (258abc9) to head (9ad05c1).
Report is 11 commits behind head on develop.

Files with missing lines	Patch %	Lines
petab/v1/distributions.py	80.00%	17 Missing and 2 partials ⚠️
petab/v1/priors.py	89.47%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #335      +/-   ##
===========================================
- Coverage    74.50%   74.08%   -0.42%     
===========================================
  Files           58       58              
  Lines         6142     6221      +79     
  Branches      1062     1078      +16     
===========================================
+ Hits          4576     4609      +33     
- Misses        1158     1206      +48     
+ Partials       408      406       -2     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dilpath]

Move this to some v1 subfolder? Now or later is fine. But I think priors will change a lot in v2

[dweindl]

I was thinking about moving it to https://github.com/PEtab-dev/PEtab/ at some point. It might also be helpful for non-python petab users.

[dilpath]

Sounds good!

[dweindl]

This does not yet address the issue of whether all priors are defined in the linear scale or the parameter scale PEtab-dev/PEtab#402.

[m-philipps]

Looks good to me, thank you! 👍

[dilpath]

I just found scipy.stats.truncate. With this, scipy.stats.norm provides all methods implemented here I think, except for the log10 chain rule factor.

However, scipy.stats.laplace only has the ppf for the cdf, not the ppf for the logcdf.

But overall it might be simpler for maintenance if we just use scipy methods, and add the few missing parts. Did you already consider this?

I'm fine with reviewing then merging the current approach too, if you prefer.

[dweindl]

I just found scipy.stats.truncate. With this, scipy.stats.norm provides all methods implemented here I think, except for the log10 chain rule factor.

However, scipy.stats.laplace only has the ppf for the cdf, not the ppf for the logcdf.

But overall it might be simpler for maintenance if we just use scipy methods, and add the few missing parts. Did you already consider this?

Yeah, initially I was also aiming to use more of scipy (or ideally only scipy), but I think that would make the implementation more patchy than now. We still would have had to implement the same functionality that is here now to fill the gaps, and would require more subclasses on top of that. Or we would have an interface that is less PEtab-compatible, because some distribution parameters are different in scipy.
This way, I found it more coherent and easier to add additional distributions as now required for PEtab 2.0 (PEtab-dev/PEtab#595, #374).

I'm fine with reviewing then merging the current approach too, if you prefer.

I would go for the current implementation, but I am open to revisit that when things in scipy.stats change.

[dilpath]

Looks great!

Just some small details/error handling as discussed