Validator: check for positive bounds for log-scaled parameter#278
Validator: check for positive bounds for log-scaled parameter#278dweindl merged 7 commits intoPEtab-dev:developfrom
Conversation
Check the parameter table for positive bounds for log-scaled estimated parameters that don't have an explicit intialization prior. See discussion in PEtab-dev#259 Supersedes and closes PEtab-dev#259
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #278 +/- ##
===========================================
- Coverage 75.68% 75.65% -0.03%
===========================================
Files 40 40
Lines 4137 4141 +4
Branches 891 893 +2
===========================================
+ Hits 3131 3133 +2
- Misses 744 745 +1
- Partials 262 263 +1 ☔ View full report in Codecov by Sentry. |
| if ( | ||
| row.get(PARAMETER_SCALE, LIN) in [LOG, LOG10] | ||
| and (row[LOWER_BOUND] == 0.0 or row[UPPER_BOUND] == 0.0) | ||
| and not row.get(INITIALIZATION_PRIOR_TYPE) | ||
| ): |
There was a problem hiding this comment.
I would move row[UPPER_BOUND] == 0.0 above, i.e. change row[UPPER_BOUND] < 0.0 to row[UPPER_BOUND] <= 0.0 -- does not make sense to estimate something that can only take the value 0.0.
There was a problem hiding this comment.
lb=ub is allowed so far. I'd say things that are legal but dumb shouldn't raise exceptions. Although we might want to add some optional hints for improvement at some point.
There was a problem hiding this comment.
Fine for me 👍 Should we check the initialization prior then, to make sure it's sampling in finite bounds?
3 participants
Check the parameter table for positive bounds for log-scaled estimated parameters that don't have an explicit intialization prior.
See discussion in #259
Supersedes and closes #259