ECP-copa / Cabana

Performance-portable library for particle-based simulations
Other
197 stars 51 forks source link

Preparation to include non equi block partitioning by liball. #439

Closed aetx closed 2 years ago

aetx commented 2 years ago

Very rough changes for now that are required by the corresponding all_lb branch of ExaMPM.

At the moment it allows one to get a mutable version of the global grid from the local grid. The global grid also allows to change the global cell offset and number of owned cells. Basically allowing to change the position and extent of the local grid.

(This also needs clean up, since not all changes are required and still remnants of tests)

junghans commented 2 years ago

format this please

codecov[bot] commented 2 years ago

Codecov Report

Merging #439 (7d05863) into master (183475e) will increase coverage by 0.0%. The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #439   +/-   ##
======================================
  Coverage    93.0%   93.0%           
======================================
  Files          37      37           
  Lines        4277    4286    +9     
======================================
+ Hits         3981    3990    +9     
  Misses        296     296           
Impacted Files Coverage Δ
cajita/src/Cajita_GlobalGrid_impl.hpp 100.0% <100.0%> (ø)
cajita/src/Cajita_LocalGrid_impl.hpp 98.0% <100.0%> (+<0.1%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 183475e...7d05863. Read the comment docs.

aetx commented 2 years ago

I am unsure what to test exactly. Just that setOwnedNumCell(i, N); ownedNumCell(i) == N;?

aetx commented 2 years ago

I also don't particularly like the mutable, but am unsure how to set the two variables without it.

streeve commented 2 years ago

I am unsure what to test exactly. Just that setOwnedNumCell(i, N); ownedNumCell(i) == N;?

Yeah, that is a really simple test. But also testing that the mutable grid is in fact mutable

aetx commented 2 years ago

I have added some tests, but am unsure how to test the mutability. Simply accessing the setter of mutGlobalGrid does not seem to trigger a failure, even though it does in ExaMPM.

junghans commented 2 years ago
The following tests FAILED:
     24 - Cajita_GlobalGrid_test_SERIAL_np_1 (Failed)
Errors while running CTest
     25 - Cajita_GlobalGrid_test_SERIAL_np_2 (Failed)
aetx commented 2 years ago

Yes, with b85213c the GlobalGrid test and LocalGrid are supposed to fail to test the test. However, LocalGrid does not seem to fail and I am unsure how to properly test the mutability of mutGlobalGrid.

junghans commented 2 years ago

Yes, with b85213c the GlobalGrid test and LocalGrid are supposed to fail to test the test. However, LocalGrid does not seem to fail and I am unsure how to properly test the mutability of mutGlobalGrid.

So what are the next step here?

aetx commented 2 years ago

These tests should now check mutability of mutGlobalGrid and the setting of the global offset and owned number of cells.

If the tests should be more rigorous let me know. Otherwise this should be done.

junghans commented 2 years ago

@aetx there is a warning now that something isn't documented in doxygen:

2021-07-29T15:23:06.2469163Z Generating docs for compound Cabana::CheckMemberTypes...
2021-07-29T15:23:06.3065749Z /__w/Cabana/Cabana/cajita/src/Cajita_GlobalGrid.hpp:160: warning: argument 'dim' of command @param is not found in the argument list of Cajita::GlobalGrid< MeshType >::setOwnedNumCell(const std::array< int, num_space_dim > &num_cell)
2021-07-29T15:23:06.3153716Z Generating docs for compound Cabana::CheckMemberTypes< MemberTypes< Types... > >...
junghans commented 2 years ago

PS: @streeve doxygen Werror worked ;-)

aetx commented 2 years ago

Ah yes, forgot to update the documentation for that.

(Working on the test case for computeGlobalOffset)