Support PEtabs initializationPrior fix #534 #535
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #535 +/- ##
===========================================
- Coverage 90.74% 90.58% -0.17%
===========================================
Files 65 65
Lines 4214 4226 +12
===========================================
+ Hits 3824 3828 +4
- Misses 390 398 +8
Continue to review full report at Codecov.
|
|
Ok, I will have a look at the Code Coverage after lunch... :) |
Seems to be a bug in the first image shown. So everything seems fine from that side :) (Tested it locally on an example) |
pypesto/petab/importer.py
Outdated
| if petab.INITIALIZATION_PRIOR_TYPE \ | ||
| not in self.petab_problem.parameter_df: | ||
| return None | ||
| else: |
There was a problem hiding this comment.
Remove unnecessary elseafter return
pypesto/problem.py
Outdated
| startpoint_method: | ||
| Method used for generating initial values for the optimization. |
There was a problem hiding this comment.
Please document expected arguments / return type
pypesto/petab/importer.py
Outdated
| not in self.petab_problem.parameter_df: | ||
| return None | ||
| else: | ||
| def startpoint_method(n_samples: int, **kwargs): |
There was a problem hiding this comment.
yep. This was included, to make the interface consistent with the other start point methods, which have different key word arguments, depending on the method.
There was a problem hiding this comment.
(Which again is needed inside of assign_startpoints, that uses an unified interface for all those methods... as far as I understand)
| @@ -64,7 +64,14 @@ def minimize( | |||
| optimizer = ScipyOptimizer() | |||
|
|
|||
| # startpoint method | |||
| if startpoint_method is None: | |||
| if (startpoint_method is not None) \ | |||
There was a problem hiding this comment.
I would just have the user-provided startpoint method have precedence over the problem-provided one. This should ensure backwards compatibility and means we don't need two defaults in the problem and here.
There was a problem hiding this comment.
👍 I will change the Error out a warning.
| not in self.petab_problem.parameter_df: | ||
| return None | ||
|
|
||
| def startpoint_method(n_samples: int, **kwargs): |
There was a problem hiding this comment.
@jvanhoefer : shouldn't that be n_starts? Seems like that is what's expected in pypesto.startpoint.util.assign_startpoints.
Seems not to be covered by tests.
No description provided.