Skip to content

Rename GC_ENABLE_GPU to GC_ENABLE_IMEX & clean up properties usage#205

Closed
kurapov-peter wants to merge 1 commit intomainfrom
pakurapo/imex-cleanup
Closed

Rename GC_ENABLE_GPU to GC_ENABLE_IMEX & clean up properties usage#205
kurapov-peter wants to merge 1 commit intomainfrom
pakurapo/imex-cleanup

Conversation

@kurapov-peter
Copy link

This cleans up the gpu passes target and prepares for the default pipeline.

@Menooker
Copy link

Menooker commented Aug 1, 2024

Overall LGTM. Do we need GC_ENABLE_GPU in the future, when we introduce non-IMEX GPU features?

@@ -1,2 +1,2 @@
if not config.gc_use_gpu:
if not config.gc_use_imex:
Copy link

Choose a reason for hiding this comment

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

This subdirectory was originally designed for general GPU tests. Shall we rename this directory to IMEX? Or maybe we can add a new subdirectory in test/gc/transforms/GPU/IMEX ?

Copy link
Author

Choose a reason for hiding this comment

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

Right, so it should either be less strict and allow some of the tests to run. Another option is to split the tests into different folders. Let's decide once we have them.

@@ -1,2 +1,2 @@
if not config.gc_use_gpu:
if not config.gc_use_imex:
Copy link

Choose a reason for hiding this comment

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

Similar to above... Do we need to distinguish general GPU tests from IMEX tests?

@kurapov-peter
Copy link
Author

Overall LGTM. Do we need GC_ENABLE_GPU in the future, when we introduce non-IMEX GPU features?

I don't yet see a need for that unless it steers some runtime dependencies, unavailable on a cpu-only machine.

@kurapov-peter
Copy link
Author

This is partially done in #200 and will be completed in #202

@kurapov-peter kurapov-peter deleted the pakurapo/imex-cleanup branch August 2, 2024 14:03
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