-
Notifications
You must be signed in to change notification settings - Fork 230
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
compiler: Generate fminf/fmaxf where necessary #2501
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -562,6 +562,31 @@ def test_minmax(): | |
assert np.all(f.data == 4) | ||
|
||
|
||
@pytest.mark.parametrize('dtype,expected', [ | ||
(np.float32, ("fmaxf", "fminf")), | ||
(np.float64, ("fmax", "fmin")), | ||
]) | ||
def test_minmax_precision(dtype, expected): | ||
grid = Grid(shape=(5, 5), dtype=dtype) | ||
|
||
f = Function(name="f", grid=grid) | ||
g = Function(name="g", grid=grid) | ||
|
||
eqn = Eq(f, Min(g, 4.0) + Max(g, 2.0)) | ||
|
||
op = Operator(eqn) | ||
|
||
g.data[:] = 3.0 | ||
|
||
op.apply() | ||
|
||
# Check generated code -- ensure it's using the fp64 versions of min/max, | ||
# that is fminf/fmaxf | ||
assert all(i in str(op) for i in expected) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be good (belt and braces) to also check: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. overkill I'd say... |
||
|
||
assert np.all(f.data == 6.0) | ||
|
||
|
||
class TestRelationsWithAssumptions: | ||
|
||
def test_multibounds_op(self): | ||
|
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.
Would use numpy numbers to check dtype is correct
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.
wdym exactly? I can think of at least two ways...
I landed on the above because that's the natural way people write the equations and it doesn't matter whether it's 4, 4.0, or 4.0F, because in the end,
g
will drive thefmin/fmax
generationThere 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.
@mloubout I'm gonna merge this because I don't like broken CI, but as soon as you elaborate on how you want this test improved, I'll mkae the change and push it together with one of the upcoming branches (at least one more to go in before christmas)