Conversation
…orked on copying over kokkos tests to test cabana
|
@Joshua-Kloepfer Thanks for addressing the items. Can you describe how the recursive baseType struct for defining ValArray works? |
cwsmith
left a comment
cwsmith
left a comment
There was a problem hiding this comment.
Looks good. A few comments are below.
|
@Joshua-Kloepfer Please address the few comments above when you get a chance. Thanks for adding the the |
|
@Joshua-Kloepfer Would you please take a look at the failing CI tests and try to fix those? |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| using Ctrlr = Controller<MemorySpace, ExecutionSpace, DataType ***>; | ||
| const int numComp = meshInfo.dim; | ||
| Ctrlr kk_ctrl({/*field 0*/ 1, numComp, meshInfo.numVtx}); | ||
| // FIXME Oversized cabana datatypes when numComp = 1|2 |
Check notice
Code scanning / CodeQL
FIXME comment Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 months ago
To fix the problem, we need to ensure that the Cabana datatypes are appropriately sized when numComp is 1 or 2. This involves modifying the type used for the Ctrlr alias and the way the controller is instantiated. Specifically, we should use a different datatype for the controller when numComp is 1 or 2 to avoid oversized allocations.
- Modify the
Ctrlralias to use a different datatype whennumCompis 1 or 2. - Update the
createControllerlambda function to handle these cases correctly. - Ensure that the changes are only applied when
MESHFIELDS_ENABLE_CABANAis defined.
| @@ -277,3 +277,3 @@ | ||
| const int numComp = meshInfo.dim; | ||
| // FIXME Oversized cabana datatypes when numComp = 1|2 | ||
|
|
||
| #ifdef MESHFIELDS_ENABLE_CABANA | ||
| @@ -283,3 +283,6 @@ | ||
| MeshField::CabanaController<ExecutionSpace, MemorySpace, DataType>>, | ||
| Controller<ExecutionSpace, MemorySpace, DataType[1][3]>, | ||
| std::conditional_t< | ||
| (numComp == 1 || numComp == 2), | ||
| Controller<ExecutionSpace, MemorySpace, DataType[1][3]>, | ||
| Controller<ExecutionSpace, MemorySpace, DataType ***>>, | ||
| Controller<MemorySpace, ExecutionSpace, DataType ***>>; | ||
| @@ -292,3 +295,7 @@ | ||
| } else { | ||
| return Ctrlr({/*field 0*/ numVtx, 1, numComp}); | ||
| if (numComp == 1 || numComp == 2) { | ||
| return Ctrlr({/*field 0*/ numVtx, 1, 3}); | ||
| } else { | ||
| return Ctrlr({/*field 0*/ numVtx, 1, numComp}); | ||
| } | ||
| } |
|
@cwsmith The CI tests were failing because the Cabana Controller is referenced even when MESHFIELDS_ENABLE_CABANA is undefined. Previously, if undefined, it would not include the Cabana Controller but would still have code using it. I surrounded code using the Cabana Controller in the shape field and omegah element tests with the same preprocessor directive and all the CI tests pass now. |
cwsmith
left a comment
cwsmith
left a comment
There was a problem hiding this comment.
Looks great. Thank you. A few comments are below.
2 participants
This PR adds support for the Cabana backend for field creation through the high level/user API.
Todo:
baseTypestruct does for definingValArraycreateCoordinateField(...)and just useDataType[1][3]in the cab controllernumComps == 1|2increateCoordinateField(...)