-
Notifications
You must be signed in to change notification settings - Fork 83
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
fix: Cast non-hashable types to floats in toms748_scan #2467
base: main
Are you sure you want to change the base?
fix: Cast non-hashable types to floats in toms748_scan #2467
Conversation
* Adding tensorflow tests are very slow
@@ -72,18 +73,20 @@ def test_toms748_scan_bounds_extension(hypotest_args): | |||
data, model, 3, 5, rtol=1e-8 | |||
) | |||
|
|||
assert observed_limit == pytest.approx(observed_limit_default) | |||
# FIXME: Remove float cast after TensorFlow support removed |
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.
@kratsg Running the TensorFlow tests is very slow and adds minutes to this run. As we're going to be dropping TensorFlow in the future, I think that we could probably just skip TensorFlow in these tests. Thoughts?
CI is currently failing for Apple silicon macOS. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2467 +/- ##
=======================================
Coverage 98.21% 98.21%
=======================================
Files 69 69
Lines 4543 4546 +3
Branches 804 805 +1
=======================================
+ Hits 4462 4465 +3
Misses 48 48
Partials 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -15,7 +15,8 @@ def __dir__(): | |||
|
|||
def _interp(x, xp, fp): |
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.
def _interp(x, xp, fp): | |
def _interp(x, xp: list[float], fp): |
@@ -15,7 +15,8 @@ def __dir__(): | |||
|
|||
def _interp(x, xp, fp): | |||
tb, _ = get_backend() | |||
return tb.astensor(np.interp(x, xp.tolist(), fp.tolist())) | |||
# xp has already been turned into a list at this point |
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.
can drop this comment with a typehint above as suggested
Description
Resolves #2466
As
toms748_scan
uses caching of results it requires that the results are hashable objects, which arrays are not. So to keep using this approach convert to floats as needed.Add testing for all backends for the upper limits inference tests.
Checklist Before Requesting Reviewer
Before Merging
For the PR Assignees: