Skip to content
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: fix non arithmetic distances #2165

Merged
merged 1 commit into from
Jul 21, 2023
Merged

compiler: fix non arithmetic distances #2165

merged 1 commit into from
Jul 21, 2023

Conversation

mloubout
Copy link
Contributor

Fix #2163

@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Merging #2165 (9d80e64) into master (a4ea4a1) will increase coverage by 0.00%.
The diff coverage is 76.92%.

@@           Coverage Diff           @@
##           master    #2165   +/-   ##
=======================================
  Coverage   87.04%   87.04%           
=======================================
  Files         223      223           
  Lines       39946    39957   +11     
  Branches     7295     7295           
=======================================
+ Hits        34769    34781   +12     
+ Misses       4598     4597    -1     
  Partials      579      579           
Impacted Files Coverage Δ
devito/passes/clusters/aliases.py 97.62% <50.00%> (+0.15%) ⬆️
tests/test_dse.py 99.79% <100.00%> (+<0.01%) ⬆️

@mloubout mloubout requested a review from FabioLuporini July 20, 2023 13:33
try:
ret[d][0] = min(ret[d][0], value)
ret[d][1] = max(ret[d][1], value)
except TypeError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, why can we now end up here with SymPy?

Copy link
Contributor

@FabioLuporini FabioLuporini Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean SymPy 1.12 and not SymPy 1.11 for example

Copy link
Contributor Author

@mloubout mloubout Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow previous versions end up with {0, expr} asv and therefore 0 as value but with sympy 1.12 you only get {expr} which breaks mon/max. Didn't look too deep into why though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there might be a deeper issue then; are we now generating the same exact code for the MFE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are we test all sympy version 1.9-1.12 that pass the same test

@FabioLuporini FabioLuporini merged commit a1da72e into master Jul 21, 2023
@FabioLuporini
Copy link
Contributor

Thanks, merged

@FabioLuporini FabioLuporini deleted the fix-2163 branch July 21, 2023 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants