-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Added parameter entry points #15
Conversation
46b7ca3
to
5374988
Compare
@arjxn-py @brosaplanella could you please review this PR? |
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 work, added some additional comments to Agriya's.
d73b574
to
601df87
Compare
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 @santacodes, it looks good. I've suggested some improvements. Feel free to let me know in case of something conflicting
Co-authored-by: Arjun Verma <[email protected]>
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.
Just a small comment, otherwise happy to merge if Agriya and Arjun are happy with it too.
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 to me too, a few final comments! Thanks, @santacodes!
Co-authored-by: Agriya Khetarpal <[email protected]>
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 @santacodes, merging this as you must have also started with model entry points so didn't want to hold this.
I hope it didn't raise any bottlenecks.
Additions -
parameter_sets.py
from the main PyBaMM repository which acts as an entry point for the parameter sets mapped through the groupcookie_parameter_sets
.Chen2020.py
to test and load a parameter set through the entry point to ensure that everything works.Updates -
dev
session.pyproject.toml
.Accessing parameter sets through entry points here is the same as accessing it through PyBaMM. Instead using
pybamm.parameter_sets
here it would implypybamm_cookiecutter.paramter_sets
to access them. I think it would make it easier for current users to adapt to the template as the implementations are almost the same. Before release, the template should be modified in compliance with the module name a user would give.