Fix startpoint sampling for PEtab-derived problems with fixed parameters

[dweindl]

Startpoint sampling for PetabImporter-derived problems didn't work correctly in case any parameters were fixed in addition to those marked estimate=0 in the underlying PEtab problem.

Fixing any parameters after the construction of the pypesto.Problem and the corresponding startpoint method would lead to errors during startpoint sampling because the list of fixed parameters was never updated.

In order to fix that, we need to have the current pypesto.Problem available for startpoint sampling to get access to the currently fixed parameters. Accessing pypesto.Problem is not compatible with the current FunctionStartpoints. Therefore, we derive a new PetabStartpoints class from CheckedStartpoints.sample that will allow forwarding/accessing the Problem.

[codecov-commenter]

Codecov Report

Merging #1169 (d4cc00d) into develop (160c2a8) will decrease coverage by 6.44%.
Report is 412 commits behind head on develop.
The diff coverage is 87.54%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           develop    #1169      +/-   ##
===========================================
- Coverage    88.16%   81.73%   -6.44%     
===========================================
  Files           79      148      +69     
  Lines         5257    11693    +6436     
===========================================
+ Hits          4635     9557    +4922     
- Misses         622     2136    +1514     

Files	Coverage Δ
pypesto/C.py	100.00% <100.00%> (ø)
pypesto/__init__.py	100.00% <100.00%> (ø)
pypesto/engine/__init__.py	100.00% <100.00%> (ø)
pypesto/engine/multi_process.py	93.10% <100.00%> (+0.79%)	⬆️
pypesto/engine/multi_thread.py	100.00% <100.00%> (ø)
pypesto/engine/single_core.py	100.00% <100.00%> (ø)
pypesto/engine/task.py	100.00% <100.00%> (ø)
pypesto/ensemble/__init__.py	100.00% <100.00%> (ø)
pypesto/ensemble/task.py	100.00% <100.00%> (ø)
pypesto/hierarchical/__init__.py	100.00% <100.00%> (ø)
... and 138 more

[FFroehlich]

I would prefer if this was solved by harmonizing pypesto_problem.x_free_indices with parameter_df[ESTIMATE] as having two distinct locations where parameter can be fixed is bound to be confusing. What happens if a parameter is fixed in petab but not pypesto_problem.x_free_indices? What happens if different values are specified in petab/pypesto?

Could be implemented by implementing setters for pypesto_problem.x_free_indices that alter the petab problem (if one is present).

[dweindl]

After PetabImporter.create_problem there isn't really a PEtab problem anymore, just the pypesto.Problem. So at this stage, we already have only a single location for fixing parameters. But any interaction with the PetabImporter.PetabProblem will not changed the fixed parameters in the previously created pypesto.Problem. So I think that is fine and pretty much what you are asking for.

The problem to be addressed here is that for the start point sampling we currently still need to look up the initialization priors in the petab.Problem enclosed in the startpoint method (which is not really accessible to the user for changing anything). And there we need to check for the actually non-fixed parameters (i.e. pypesto.Problem), not for those PEtab-non-fixed prarameters.

The alternative would be creating a new class PetabProblem(pypesto.Problem) that as opposed to now, keeps a petab.Problem, syncs pypesto.Problem.x_free_indices and PetabProblem.parameter_df.estimate, and shares that with the startpoint method/class. Could be done, but I am not sure that's really preferable.

[dweindl]

Okay, what could be done to make things look slightly nicer: Instead of checking for PEtab-estimate and x_free_indices in order to subset the result of petab.sample_parameter_startpoints, I can add a parameters: Sequence[str] argument to petab.sample_parameter_startpoints that subsets and orders things as required. (Or implement such a method here in pypesto, as a faster solution.) It doesn't really change much overall, but PetabStartpoints.sample will be simpler.

EDIT: did that. or something similar.

[PaulJonasJost]

Thanks for fixing this. I do agree with your argument, that basically we do not use the petab.Problem anymore after the importer. Therefore I think this is a reasonable solution.

[FFroehlich]

Thanks!