Rewriter: fix fuse_relu_clip with None max

[AyoubMDL]

Fixes #2836

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.78%. Comparing base (1350ed2) to head (e4335c4).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2843      +/-   ##
==========================================
- Coverage   71.79%   71.78%   -0.01%     
==========================================
  Files         239      239              
  Lines       29019    29020       +1     
  Branches     2864     2864              
==========================================
- Hits        20834    20833       -1     
- Misses       7213     7216       +3     
+ Partials      972      971       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gramalingam]

I don't see a check here whether max_clip.const_value is None. It is possible that it is being checked elsewhere before this method if called, but, even so, it would be better if all related logic is in one place (checking if an input is present, whether it is a constant, etc.). Overall, it feels like the structure of this implementation turns what should be a mathematically simple rule (easy to read and validate) into something opaque.

I realize that this is mostly a problem with the pre-existing rule, but it just makes updates like this harder to review.

[AyoubMDL]

I agree with the general point, but I think this case is a bit special. Normally we would perform these checks in the check method and fail the pattern if they don’t pass. However, in this pattern the max/min values can be None, and the rewrite can still proceed in that case. That’s why the logic was isolated in extract_min_max method instead of being handled entirely in check.
That said, I’m open to suggestions if you think there’s a cleaner way to structure this.

[AyoubMDL]

@justinchuby Test fails because the onnx=1.17 (from noxfile) doesnt allow to define clip this way (Clip(x1,min,"")). Later versions allow it.