-
Notifications
You must be signed in to change notification settings - Fork 4
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
Compatibility with (future) pymbolic 2024.1 #27
Comments
Thanks for the heads up @inducer. I will mention the 3.10 concerns with the group this Wednesday and let you know. |
One issue I've just encountered is until now we have been using the latest release of pymbolic (2022.2). This breaks with current loopy |
This restores compatibility with pymbolic 2022.2 x-ref: firedrakeproject/loopy#27
This restores compatibility with pymbolic 2022.2 x-ref: firedrakeproject/loopy#27
Thanks for flagging this, that was an (unnecessary) oversight. AFAICT, inducer/loopy#874 restores compatibility with pymbolic 2022.2, so the updated suggestion would be to pull in whatever that becomes. |
The driving factor behind this is that mappers now inherit from |
This restores compatibility with pymbolic 2022.2 x-ref: firedrakeproject/loopy#27
|
Thanks for this. I will refresh the loopy update PR to see if things now work and get that merged shortly. Following some discussion in today's meeting we have decided that we are happy to drop support for 3.9 at the same time as you. |
@inducer just to let you know that the version of loopy we just merged is in fact not compatible with Python 3.9: #28 (comment). This is fine for us as we agreed to shift to 3.10+ anyway. |
@connorjward Could you guys pull inducer/loopy@7ac9fa6 into Firedrake? pymbolic is going through a bit of a remodel, with inducer/pymbolic#152 and inducer/pymbolic#125.
The linked version will be compatible with both old-timey and new-timey pymbolic. New-timey pymbolic is mostly inducer/pymbolic#152, which, in the near future, will become pymbolic 2024.1. As a side note, pymbolic 2024.1 will require Python 3.10. Loopy will likely follow suit in the near future. I know you all are still targeting 3.9, so if that's an issue, let's discuss.
(The currently-broken mypy in Loopy is the result of more precise typing information from inducer/pymbolic#125, in case you're wondering.)
Things are generally compatible, however inducer/pymbolic#152 introduces one long-overdue behavior change:
0 + x
is nowSum(0, x)
, notx
. Callpymbolic.flatten
to getx
. This change should be semantics-neutral, and AFAIK nothing in Loopy actually cares. Nonetheless, some of loopy's tests want specific forms of some expressions, so the linked version of loopy sprinkles some calls toflatten
.The text was updated successfully, but these errors were encountered: