Open
Conversation
JustinPrivitera
commented
Apr 16, 2026
Member
Author
|
Running on device performance annotations: |
Member
Author
|
Running with much larger arrays, src starting on host des starting on device |
JustinPrivitera
commented
Apr 24, 2026
|
|
||
| // Compute face centers and normals. | ||
| conduit::execution::for_all<ExecPolicy>(0, totalNumFaces, [=](conduit::index_t f) { | ||
| conduit::execution::forall<ExecPolicy>(0, totalNumFaces, [=](conduit::index_t f) { |
Member
Author
There was a problem hiding this comment.
I am confused by this
Member
Author
There was a problem hiding this comment.
wherever this method is called, we want to take a look at the template and consider switching to a policy object choice.
JustinPrivitera
commented
Apr 24, 2026
Member
Author
There was a problem hiding this comment.
look into RDC before getting Cyrus' blessing
JustinPrivitera
commented
Apr 24, 2026
JustinPrivitera
commented
Apr 24, 2026
JustinPrivitera
commented
Apr 24, 2026
JustinPrivitera
commented
Apr 24, 2026
| bool | ||
| ExecutionPolicy::is_parallel_policy() const | ||
| { | ||
| return is_cuda() || is_hip() || is_openmp(); |
Member
Author
There was a problem hiding this comment.
to look at in the future - we don't have consistency here.
You can create a "parallel" exec policy that is_parallel_policy() returns false for. We should make a ticket to examine in the future.
Member
Author
There was a problem hiding this comment.
maybe a note in the code too.
Member
Author
There was a problem hiding this comment.
I added the note in the code; we just need a ticket with follow-on work now.
JustinPrivitera
commented
Apr 24, 2026
| }; | ||
|
|
||
| //----------------------------------------------------------------------------- | ||
| void init_device_memory_handlers(); |
Member
Author
There was a problem hiding this comment.
this should be a static function that people only have to do once. We should have a conduit utility somewhere that should be able to do this somewhere. It should see if the handlers are installed and do it, otherwise do nothing.
Member
Author
There was a problem hiding this comment.
could be a TODO for later
JustinPrivitera
commented
Apr 24, 2026
JustinPrivitera
commented
Apr 24, 2026
JustinPrivitera
commented
Apr 24, 2026
JustinPrivitera
commented
Apr 24, 2026
Comment on lines
+22
to
+47
| // Build capability answers "was this Conduit build configured with a RAJA | ||
| // backend?", while TU capability answers "is this translation unit actually | ||
| // being compiled with that device compiler right now?" | ||
| #if defined(CONDUIT_USE_RAJA) && defined(CONDUIT_USE_CUDA) | ||
| #define CONDUIT_EXEC_BUILD_HAS_CUDA | ||
| #endif | ||
|
|
||
| #if defined(CONDUIT_USE_RAJA) && defined(CONDUIT_USE_HIP) | ||
| #define CONDUIT_EXEC_BUILD_HAS_HIP | ||
| #endif | ||
|
|
||
| #if defined(CONDUIT_EXEC_BUILD_HAS_CUDA) || defined(CONDUIT_EXEC_BUILD_HAS_HIP) | ||
| #define CONDUIT_EXEC_BUILD_HAS_DEVICE | ||
| #endif | ||
|
|
||
| #if defined(CONDUIT_EXEC_BUILD_HAS_CUDA) && defined(__CUDACC__) | ||
| #define CONDUIT_EXEC_TU_HAS_CUDA | ||
| #endif | ||
|
|
||
| #if defined(CONDUIT_EXEC_BUILD_HAS_HIP) && defined(__HIPCC__) | ||
| #define CONDUIT_EXEC_TU_HAS_HIP | ||
| #endif | ||
|
|
||
| #if defined(CONDUIT_EXEC_TU_HAS_CUDA) || defined(CONDUIT_EXEC_TU_HAS_HIP) | ||
| #define CONDUIT_EXEC_TU_HAS_DEVICE | ||
| #endif |
Member
Author
There was a problem hiding this comment.
codex disaster
we will never have to worry about cuda or hip
we have duplicated this in the conduit_execution.hpp header
we want to use what is there and not have all this madness
JustinPrivitera
commented
Apr 24, 2026
Comment on lines
+454
to
+481
| inline void | ||
| validate_runtime_policy(const ExecutionPolicy &policy, | ||
| const char *context) | ||
| { | ||
| if (policy.is_empty()) | ||
| { | ||
| CONDUIT_ERROR(context << " does not support an empty policy."); | ||
| } | ||
|
|
||
| if (policy.is_openmp()) | ||
| { | ||
| #if !defined(CONDUIT_USE_OPENMP) | ||
| CONDUIT_ERROR(context << " requires OpenMP support in this translation unit."); | ||
| #endif | ||
| } | ||
| else if (policy.is_cuda()) | ||
| { | ||
| #if !defined(CONDUIT_EXEC_TU_HAS_CUDA) | ||
| CONDUIT_ERROR(context << " requires CUDA support in this translation unit."); | ||
| #endif | ||
| } | ||
| else if (policy.is_hip()) | ||
| { | ||
| #if !defined(CONDUIT_EXEC_TU_HAS_HIP) | ||
| CONDUIT_ERROR(context << " requires HIP support in this translation unit."); | ||
| #endif | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
what is this and these strange ifdefs. Why do we need this?
Member
Author
There was a problem hiding this comment.
won't the policy creation fail? shouldn't it?
Member
Author
There was a problem hiding this comment.
runtime error for compile errors is strange. We should find a way to error earlier if someone wants to run on a GPU and we can't.
Member
Author
There was a problem hiding this comment.
not clear we need to validate like this right now.
JustinPrivitera
commented
Apr 24, 2026
JustinPrivitera
commented
Apr 24, 2026
| #if defined(CONDUIT_USE_OPENMP) | ||
| detail::ReduceSumImpl<OpenMPExec, T> m_openmp_reduce; | ||
| #endif | ||
| #if defined(CONDUIT_EXEC_TU_HAS_CUDA) |
Member
Author
There was a problem hiding this comment.
name is not great :/
Member
Author
|
use hatchet/thicket to compare speedup |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1358