-
Notifications
You must be signed in to change notification settings - Fork 25
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
Temperature can be given as float or sp.Expr #770
Temperature can be given as float or sp.Expr #770
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #770 +/- ##
=======================================
Coverage 99.51% 99.52%
=======================================
Files 58 58
Lines 2488 2501 +13
=======================================
+ Hits 2476 2489 +13
Misses 12 12 ☔ View full report in Codecov by Sentry. |
@T.setter | ||
def T(self, value): | ||
if isinstance(value, festim.Temperature): | ||
self._T = value | ||
elif value is None: | ||
self._T = value | ||
elif isinstance(value, (int, float, sp.Expr)): | ||
self._T = festim.Temperature(value) |
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.
Possibly, an else
case with an error can be included to add clarity? Similar to
FESTIM/festim/generic_simulation.py
Lines 124 to 127 in fb2c99f
else: | |
raise TypeError( | |
"Accepted types for traps are list, festim.Traps or festim.Trap" | |
) |
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.
added
Proposed changes
This fixes #741 by adding a setter/getter to the
T
attribute ofSimulation
.Now, users can simply give the temperature as a numerical value or sympy Expression without having to create a
Temperature
object.Little QoL improvement!
Types of changes
What types of changes does your code introduce to FESTIM?
Checklist