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

Parameter plot w/ hier. pars, noise estimation for splines #1061

Merged
merged 11 commits into from
May 11, 2023

Conversation

Doresic
Copy link
Contributor

@Doresic Doresic commented May 4, 2023

Extended the parameter plots to include hierarchical parameters. Not all, but the interpretable ones: scaling, offset, and noise. So excluding the very numerous spline/optimal scaling inner parameters.

  • The amici objective has an attribute self.inner_parameters in which it will save the best last inner parameters of the current optimization run (start point).
  • Then at the end of the optimization run, it will be saved into the optimization_result dictionary.
  • The parameter plotting routine will check for inner parameters in parameters.py/handle_inputs and concatenate them to the outputs accordingly.

Implemented the noise estimation for observables with non-linear monotone data.

  • The noise is calculated analytically, basically in the same way as with relative data.
  • To specify that the noise parameters should be optimized hierarchically, as with relative data, the parameterType column entry of the noise parameter should be sigma. Then the SplineInnerProblem will extract the noise parameters.

- Implemented the hierarchical optimization of noise for nonlinear-monotone observables.
- Cleaned up constants, and solver.
@Doresic
Copy link
Contributor Author

Doresic commented May 4, 2023

Example of what the plot looks like for the Boehm model with hierarchically estimated noise:

image

Since the scale of the hierarchical parameters is lin, it will be harder to see the kinetic parameters nicely. So I've added a flag plot_inner_parameters to the parameters plotting routine. The default is True.

@codecov-commenter
Copy link

codecov-commenter commented May 4, 2023

Codecov Report

Merging #1061 (05fa654) into develop (fad8041) will decrease coverage by 0.10%.
The diff coverage is 86.56%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##           develop    #1061      +/-   ##
===========================================
- Coverage    84.88%   84.78%   -0.10%     
===========================================
  Files          147      147              
  Lines        11252    11384     +132     
===========================================
+ Hits          9551     9652     +101     
- Misses        1701     1732      +31     
Impacted Files Coverage Δ
pypesto/visualize/ordinal_categories.py 8.63% <0.00%> (-0.04%) ⬇️
pypesto/visualize/spline_approximation.py 15.00% <0.00%> (-0.26%) ⬇️
pypesto/hierarchical/optimal_scaling/solver.py 93.03% <50.00%> (-1.44%) ⬇️
pypesto/optimize/optimizer.py 88.29% <66.66%> (-0.16%) ⬇️
pypesto/hierarchical/optimal_scaling/problem.py 96.74% <80.00%> (-0.40%) ⬇️
...sto/hierarchical/spline_approximation/parameter.py 81.81% <80.00%> (+1.81%) ⬆️
...to/hierarchical/spline_approximation/calculator.py 76.66% <83.33%> (+0.74%) ⬆️
...pesto/hierarchical/spline_approximation/problem.py 89.18% <85.36%> (-1.72%) ⬇️
...ypesto/hierarchical/spline_approximation/solver.py 83.68% <85.89%> (-0.82%) ⬇️
pypesto/visualize/parameters.py 83.73% <94.11%> (+1.18%) ⬆️
... and 7 more

... and 3 files with indirect coverage changes

Copy link
Contributor

@FFroehlich FFroehlich left a comment

Choose a reason for hiding this comment

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

Graph looks nice! Only looked at changes is pypesto/objective/amici/amici.py and pypesto/petab/importer.py

pypesto/objective/amici/amici.py Outdated Show resolved Hide resolved
pypesto/petab/importer.py Outdated Show resolved Hide resolved
Copy link
Member

@dweindl dweindl left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good as far as I can tell.

pypesto/visualize/parameters.py Outdated Show resolved Hide resolved
pypesto/visualize/parameters.py Outdated Show resolved Hide resolved
pypesto/hierarchical/spline_approximation/calculator.py Outdated Show resolved Hide resolved
pypesto/hierarchical/spline_approximation/problem.py Outdated Show resolved Hide resolved
pypesto/hierarchical/spline_approximation/solver.py Outdated Show resolved Hide resolved
test/hierarchical/test_spline.py Outdated Show resolved Hide resolved
@Doresic
Copy link
Contributor Author

Doresic commented May 8, 2023

Base test is failing very often. Specifically test_objective.py/test_finite_difference_checks():

# Test the single step size `check_grad` method.
        eps = 1e-5
        result_single_eps = objective.check_grad(np.array([theta]), eps=eps)
>       assert result_single_eps['rel_err'].squeeze() == rel_err(eps)
E       assert 1.1987971168495947e-10 == 1.1987977630586244e-10
E        +  where 1.1987971168495947e-10 = <bound method NDFrame.squeeze of x_0    1.198797e-10\nName: rel_err, dtype: float64>()
E        +    where <bound method NDFrame.squeeze of x_0    1.198797e-10\nName: rel_err, dtype: float64> = x_0    1.198797e-10\nName: rel_err, dtype:

It seems that the assert is checking whether very small numbers are equal.
Is this failing in other PRs?

@dweindl
Copy link
Member

dweindl commented May 8, 2023

Is this failing in other PRs?

Dunno, but it definitely makes sense to change that to something like numpy.testing.assert_array_almost_equal_nulp

Copy link
Collaborator

@PaulJonasJost PaulJonasJost left a comment

Choose a reason for hiding this comment

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

Looks good to me. Do we have a test for the inner parameter visualisation? Could be useful just to ensure its running later on (as other visualization tests)

@PaulJonasJost
Copy link
Collaborator

Is this failing in other PRs?

Dunno, but it definitely makes sense to change that to something like numpy.testing.assert_array_almost_equal_nulp

should be fixed in #1064

Copy link
Contributor

@stephanmg stephanmg left a comment

Choose a reason for hiding this comment

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

Looks fine. I agree on Paul's comment, a test for inner parameter visualization would be great.

Added the test for parameter plot of results of problems with inner hierarchical parameters
@Doresic
Copy link
Contributor Author

Doresic commented May 11, 2023

I've just added a test for the parameter plot with inner hierarchical parameters

@Doresic Doresic merged commit f29c848 into develop May 11, 2023
@Doresic Doresic deleted the spline_noise_estimation branch May 11, 2023 14:56
This was referenced Jun 22, 2023
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.

8 participants