-
Notifications
You must be signed in to change notification settings - Fork 152
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
Allow for the potential_fn to be a Callable #943
Conversation
e9b07b1
to
cb53e67
Compare
abb2a1f
to
770d8d3
Compare
542ea1c
to
9d4f0bb
Compare
9d4f0bb
to
2d556b0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #943 +/- ##
==========================================
+ Coverage 75.29% 75.94% +0.65%
==========================================
Files 80 80
Lines 6285 6306 +21
==========================================
+ Hits 4732 4789 +57
+ Misses 1553 1517 -36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ 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.
Looks great, thanks! Good idea to use a wrapper here.
I added a couple of questions regarding documentation and making sure the passed potential has what we need.
feaef83
to
88b1d6e
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.
Great! Just one final comment about the typing.
88b1d6e
to
db89fc7
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.
looks good and ready to go into main
. Thanks!
We now only require that the
potential_fn
takestheta
andx_o
as inputs.The newly added tests take
2.14 seconds
in total.Closes #765