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

chore: Update blt, RAJA, CHAI, Caliper, adiak, conduit #3218

Merged
merged 41 commits into from
Jul 17, 2024

Conversation

rrsettgast
Copy link
Member

@rrsettgast rrsettgast commented Jul 10, 2024

@rrsettgast rrsettgast added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Jul 10, 2024
@rrsettgast rrsettgast self-assigned this Jul 10, 2024
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.72%. Comparing base (9a144b8) to head (19c7110).
Report is 84 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3218      +/-   ##
===========================================
- Coverage    55.73%   55.72%   -0.01%     
===========================================
  Files         1041     1041              
  Lines        88562    88559       -3     
===========================================
- Hits         49360    49353       -7     
- Misses       39202    39206       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -53,7 +53,9 @@ if(EXISTS ${GEOSX_TPL_DIR}/vtk)
endif()

if(EXISTS ${GEOSX_TPL_DIR}/fmt)
set(FMT_DIR ${GEOSX_TPL_DIR}/fmt CACHE PATH "" FORCE)
# set(FMT_DIR ${GEOSX_TPL_DIR}/fmt CACHE PATH "" FORCE)
set(FMT_DIR ${GEOSX_TPL_DIR}/chai CACHE PATH "" FORCE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is FMT_DIR pointing to chai ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. there is a conflict. A lot of the TPL's have fmt integrated, and umpire which is included in CHAI changed the way they include it so now there can be a conflict....it is annoying to say the least.

message( " ----> HDF5 version ${HDF5_VERSION}")

get_target_property(HDF5_INCLUDE_DIRS HDF5::HDF5 INTERFACE_INCLUDE_DIRECTORIES)
set_target_properties(HDF5::HDF5 PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${HDF5_INCLUDE_DIRS}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to put the includes into system interface ?

Copy link
Member Author

Choose a reason for hiding this comment

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

We do this so that they are included with -isystem instead of -I to suppress any compiler warnings.

@rrsettgast rrsettgast changed the title chore: Update BLT to version 0.6.2 chore: Update BLT, RAJA, CHAI Jul 14, 2024
scripts/ci_build_and_test_in_container.sh Outdated Show resolved Hide resolved
Comment on lines 95 to 100
// events.emplace_back( forAll< parallelDeviceAsyncPolicy<> >( stream, var.size(), [=] GEOS_DEVICE ( localIndex ii )
forAll< parallelDevicePolicy<> >( var.size(), [=] GEOS_DEVICE ( localIndex ii )
{
reinterpret_cast< std::remove_const_t< T > * >( buffer )[ ii ] = var.data()[ ii ];
} ) );
} );
//);
Copy link
Member Author

@rrsettgast rrsettgast Jul 16, 2024

Choose a reason for hiding this comment

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

@wrtobin Why is this not something like:

    std::remove_const_t <T> * const lhs = reinterpret_cast< std::remove_const_t< T > * >( buffer );
   forAll< parallelDevicePolicy<> >( var.size(), [=] GEOS_DEVICE ( localIndex ii )
     {
       lhs[ ii ] = var[ ii ];
     } );

Copy link
Collaborator

@wrtobin wrtobin Jul 17, 2024

Choose a reason for hiding this comment

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

Thats a fine refactor, except we do need var.data()[ ii ] since we're packing arbitrary multidimensional slices and just want the underlying array buffer, which inside the loop is on device and outside the loop is on host.

Since the lhs is a pinned buffer moving the cast outside the loop should be fine and might be more clear, but the performance should be the same since the cast is a compile-time operation on memory location/alignment more than anything else.

@rrsettgast rrsettgast changed the title chore: Update BLT, RAJA, CHAI chore: Update blt, RAJA, CHAI, Caliper, adiak, conduit Jul 17, 2024
@rrsettgast rrsettgast merged commit 5fe43d6 into develop Jul 17, 2024
24 checks passed
@rrsettgast rrsettgast deleted the update/update_to_blt_0.6.2 branch July 17, 2024 20:19
rrsettgast added a commit that referenced this pull request Jul 18, 2024
* update blt to 0.6.2

* update RAJA and CHAI to v2024.02.2

* update Caliper to 2.11.0

*update adiak to 0.4.0

* update conduit to 0.9.2

* change to new HDF import. use umpires fmt and add extra dependency to executable

* changes for change in raja interface

* move src/coreComponents/unitTests/dataRepository/testPacking.cpp to src/coreComponents/dataRepository/unitTests

* switched to syncronous packing/upacking to avoid test failures on CI image running on streak
Algiane pushed a commit that referenced this pull request Jul 30, 2024
* update blt to 0.6.2

* update RAJA and CHAI to v2024.02.2

* update Caliper to 2.11.0

*update adiak to 0.4.0

* update conduit to 0.9.2

* change to new HDF import. use umpires fmt and add extra dependency to executable

* changes for change in raja interface

* move src/coreComponents/unitTests/dataRepository/testPacking.cpp to src/coreComponents/dataRepository/unitTests

* switched to syncronous packing/upacking to avoid test failures on CI image running on streak
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants