compiler: Generate fminf/fmaxf where necessary #2501
Conversation
FabioLuporini
added
the
bug-C-minor
FabioLuporini
requested review from
mloubout,
georgebisbas,
EdCaunt and
JDBetteridge
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2501 +/- ##
==========================================
- Coverage 87.29% 87.29% -0.01%
==========================================
Files 238 238
Lines 45382 45395 +13
Branches 4031 4032 +1
==========================================
+ Hits 39616 39627 +11
- Misses 5083 5084 +1
- Partials 683 684 +1 ☔ View full report in Codecov by Sentry. |
|
|
||
| # 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.
Would it be good (belt and braces) to also check:
assert all(i not in str(op) for i in not_expected)
to ensure that the correct versions are used throughout?
There was a problem hiding this comment.
overkill I'd say...
| f = Function(name="f", grid=grid) | ||
| g = Function(name="g", grid=grid) | ||
|
|
||
| eqn = Eq(f, Min(g, 4.0) + Max(g, 2.0)) |
There was a problem hiding this comment.
Would use numpy numbers to check dtype is correct
There was a problem hiding this comment.
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 the fmin/fmax generation
There was a problem hiding this comment.
@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)
No description provided.