-
Notifications
You must be signed in to change notification settings - Fork 47
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
Hierarchical: restructuring and add relative to InnerCalculatorCollector #1245
Conversation
Inner parameters were not being stored when the result was stored. This was due to a problem with putting a dictionary into an HDF5 format. I've transformed it into 2 lists now: a INNER_PARAMETERS_VALUES and INNER_PARAMETER_NAMES list.
- Restructuring of the hierarchical scaling+offset method. The base classes used by all non-quantitative and semi-quantitative data types are put in the `base_...` files. All classes related to the scaling+offset method (for relative data) is moved to the `relative` folder, analogous to other data types. - Renaming: the naming scheme doesn't follow the method, but the data type now. It's more direct and removes the ambiguity of `OptimalScaling` for ordinal data and `scaling_offset` for relative data. - Optimal scaling -> ordinal - Spline approximation -> semiquantitative - Scaling + offset -> relative
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1245 +/- ##
===========================================
- Coverage 84.59% 84.28% -0.32%
===========================================
Files 148 151 +3
Lines 12122 12331 +209
===========================================
+ Hits 10255 10393 +138
- Misses 1867 1938 +71 ☔ View full report in Codecov by Sentry. |
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.
Only looked at changes in pypesto/petab/importer.py
and complained about files where I don't think I should be codeowner.
doc/example/example_semiquantitative/measurementData_example_semiquantitative_linear.tsv
Show resolved
Hide resolved
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 for cleaning that up.
Smaller pull requests strongly preferred.
Looks good as far as I can tell.
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.
Looks good, and thanks for cleaning up the code. See my comments.
Is there an option to pre-view the documentation? |
Yes, it should be possible by clicking on the readthedocs test and then view Docs (if it runs successfully). @m-philipps |
Complete restructuring of the hierarchical module.
The formerly named
hierarchical
module which optimizes scaling, offset, and noise of relative data was renamed torelative
. Classes used by all non-quantitative data type hierarchical algorithms (ordinal, relative, and semiquantitative) were moved tobase_...
files, while the classes and functions related specifically to relative data (scaling and offset problem, calculator, solver etc...) were moved to the hierarchicalrelative
sub-module.Renamed the former
optimal_scaling
module used for ordinal data toordinal
to remove any confusion regarding a similar name of scaling parameters for relative data.Renamed the former
spline_approximation
module tosemiquantitative
. Now all hierarchical submodules are called after the data they integrate with rather than the method they use.A lot of Class renaming to fit the renaming already mentioned.
Included the
relative
method (scaling and offset) into the InnerCalculatorCollector. Now, one can have a PEtab problem containing all three types of observables: relative, ordinal, and semiquantitative observables.Added hierarchical PEtab validation to prevent things like specifying relative observables with semiquantitative data etc...
Fixed visualizations, docs, and tests to work with the renaming.
The
RelativeCalculator
has two operating modes:call_amici_twice
andcalculate_directly
. The former is the old implementation of the calculator. It simulates the model twice. However, this can be used only if relative data is the only non-quantitative data type. In this case, one can use 2nd order sensitivities and adjoint sensitivity analysis as before. These two are not implemented for the latter modecalculate_directly
. This mode will simulate only once, and calculate the objective function and gradient directly (but only for the relative data, not other data). It can be used only with forward sensitivity analysis since it uses sensitivities in the gradient calculation.