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

refactor[next]: move lift_mode itir test fixture into program_processor #1533

Merged
merged 13 commits into from
Apr 23, 2024

Conversation

havogt
Copy link
Contributor

@havogt havogt commented Apr 18, 2024

Currently we have a mix of specifying the backend which already comes with a lift_mode default and separately the lift_mode fixture in some tests. The default was not overwritten in the roundtrip backend. Now we remove the separate lift_mode and add extra backends with the lift_mode set.

Note: we don't run double_roundtrip with temporaries and we don't use the lift_mode==SIMPLE_HEURISTIC in roundtrip, only USE_TEMPORARIES.

Longer term we should refactor all itir tests to use the ffront test infrastructure.

@edopao
Copy link
Contributor

edopao commented Apr 19, 2024

I think with this change we can also remove a couple of TODOs and temporary workarouds:

https://github.com/GridTools/gt4py/blob/main/src/gt4py/next/program_processors/codegens/gtfn/gtfn_module.py
TODO(tehrengruber): Remove lift_mode from call interface.

https://github.com/GridTools/gt4py/blob/main/src/gt4py/next/program_processors/runners/dace_iterator/workflow.py
TODO(tehrengruber): Remove lift_mode from call interface.

@havogt
Copy link
Contributor Author

havogt commented Apr 19, 2024

Thank you! Is this

Currently only the `FORCE_INLINE` liftmode is supported and the value of `lift_mode` is ignored.
still true?

@edopao
Copy link
Contributor

edopao commented Apr 19, 2024

Thank you! Is this

Currently only the `FORCE_INLINE` liftmode is supported and the value of `lift_mode` is ignored.

still true?

No, this comment is not true anymore. It can be removed.

@havogt havogt requested a review from tehrengruber April 19, 2024 09:25
Copy link
Contributor

@tehrengruber tehrengruber left a comment

Choose a reason for hiding this comment

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

Just small things. Since LiftMode.SIMPLE_HEURISTIC is not tested anymore I would add either a comment to the heuristic or remove it.

src/gt4py/next/program_processors/runners/roundtrip.py Outdated Show resolved Hide resolved
src/gt4py/next/program_processors/runners/roundtrip.py Outdated Show resolved Hide resolved
@havogt havogt merged commit 6a5ae7e into GridTools:main Apr 23, 2024
39 checks passed
@havogt havogt deleted the test_lift_mode_to_processor branch April 23, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants