-
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
Add option for hierarchical models #1241
Conversation
Add option for hierarchical models in model selection
Fix typo
import pandas
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1241 +/- ##
===========================================
- Coverage 88.16% 82.55% -5.62%
===========================================
Files 79 148 +69
Lines 5257 11955 +6698
===========================================
+ Hits 4635 9869 +5234
- Misses 622 2086 +1464 ☔ 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.
Nice, thanks! Let's merge this after you see that it works in your case.
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
hierarchical: | ||
Whether the problem involves hierarchical optimization. |
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.
How about directly forwarding any **kwargs
to PetabImporter()
?
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.
Could be done as well.
I personally found it more transparent in this way, instead of hiding it in **kwargs
.
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.
These methods (model_to_pypesto_problem
and model_to_hierarchical_pypesto_problem
) are used as an argument [1].
A separate method was provided so that the user doesn't need to setup with partial
like
from functools import partial
model_to_pypesto_problem_method = partial(model_to_pypesto_problem, hierarchical=True)
pypesto_select_problem.select_to_completion(..., model_to_pypesto_problem_method=model_to_pypesto_problem_method)
I think forwarding **kwargs
is a good idea, but they will need to be forwarded to two calls: correct_x_guesses
and PetabImporter
[1]
pyPESTO/pypesto/select/method.py
Line 250 in b4a187f
model_to_pypesto_problem_method: Callable[[Any], Problem] = None, |
Add option for hierarchical models in model_selection.