-
Notifications
You must be signed in to change notification settings - Fork 8
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
can init mech with nexus id #113
Conversation
Change-Id: I802aa7cf0c16bda21e5bc4c0f08e44b6e93a0e52
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #113 +/- ##
=======================================
Coverage 59.13% 59.13%
=======================================
Files 106 106
Lines 7652 7652
=======================================
Hits 4525 4525
Misses 3127 3127 ☔ View full report in Codecov by Sentry. |
@@ -211,6 +211,7 @@ def init_from_dict(self, configuration_dict, morphology, auto_mechanism=False): | |||
mechanism.get("version", None), | |||
mechanism.get("temperature", None), | |||
mechanism.get("ljp_corrected", None), | |||
mechanism.get("id", None), |
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.
shouldn't we update the definition of add_mechanism
?
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.
good catch. solved in latest commit
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, then we should either call the function using keyword arguments or consider rearranging the function definition of add_mechanism
to move auto_parameter
to the last position in its parameter list
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.
Indeed. I added keyword arguments to the function call
Change-Id: I9440211e61ae69a399b897a68b904346621a4b99
So, you do not plan to add modelid for the mechanisms here? |
No, we said just nexus_id. |
Change-Id: I03ccff72af19c1c41ffc10dd7e83e640ddb036dd
The modelid could also be another entry as a fallback if nexus id is deprecated or not present. This can ensure that bpem downloads correct mechanisms. But for now, it is fine to use the nexus id. |
No description provided.