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

Arbitrary grid updates #477

Merged
merged 9 commits into from
Jan 3, 2024
Merged

Conversation

kubagalecki
Copy link
Contributor

Some updates to the arbitrary grid formulation:

  • Edges between bulk nodes removed from the connectivity graph. This reduces the communication volume and associated costs.
  • The local permutation strategy can now be set via the (optional) "permutation" attribute of the "ArbitraryLattice" xml node. Valid values are:
    • "none" - use default ordering, not recommended, only for debugging/benchmarking purposes
    • "coords" - sort only by coordinates, this was the previous behavior. This is still the default.
    • "type" - sort only by node type, not recommended, only for debugging/benchmarking purposes
    • "both" - sort by node type, subsort by coordinates. Somewhat surprisingly, for the fontaine benchmark (on a single GPU), this resulted in a ~8.5% performance penalty, meaning less coalesced memory access outweighed the gains from more efficient warp utilization. I still think for some cases this might be more efficient than the "coords" strategy (maybe for very low porosity, or when the geometry is more structured?)
  • Added some missing handlers

Outstanding items:

  • Symmetry conditions
  • Some handlers are still missing
    • Mostly related to things like particles, so implementing them is outside the scope of this project
    • RunR is definitely still broken
    • Failcheck semantics make little sense for arbitrary grid, see Failcheck always triggers for arbitrary grid #476
    • You can check which handlers still need implementing by grepping for getCartLattice
  • Adjoint functionality is definitely broken, the main iteration is missing as well as saving/loading components. There are probably also a bunch of other places where I screwed something up, but I don't really have the bandwidth to go hunting for them.

- edges between bulk nodes in the connectivity graph are now ignored (consequential for iteration communication volume and partitioning quality)
- toArb memory requirement w.r.t. bounding box size reduced by half (now just 1 bit per voxel)
- minor refactoring
@codecov-commenter
Copy link

codecov-commenter commented Dec 31, 2023

Codecov Report

Attention: 421 lines in your changes are missing coverage. Please review.

Comparison is base (b38b8dd) 38.09% compared to head (e02f071) 37.72%.

Files Patch % Lines
src/ArbLattice.cpp 0.00% 146 Missing ⚠️
src/vtkLattice.cpp 13.33% 65 Missing ⚠️
src/Handlers/cbFailcheck.cpp 0.00% 54 Missing ⚠️
src/toArb.cpp 0.00% 44 Missing ⚠️
src/Handlers/cbSaveCheckpoint.cpp 0.00% 40 Missing ⚠️
src/Handlers/cbSaveBinary.cpp 0.00% 16 Missing ⚠️
src/Handlers/cbSaveMemoryDump.cpp 0.00% 15 Missing ⚠️
src/Handlers/cbTXT.cpp 0.00% 13 Missing ⚠️
src/Handlers/acLoadMemoryDump.cpp 0.00% 11 Missing ⚠️
src/Handlers/cbBIN.cpp 0.00% 9 Missing ⚠️
... and 2 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #477      +/-   ##
============================================
- Coverage     38.09%   37.72%   -0.38%     
============================================
  Files           185      185              
  Lines          8355     8461     +106     
============================================
+ Hits           3183     3192       +9     
- Misses         5172     5269      +97     
Flag Coverage Δ
d2q9 26.38% <2.32%> (-0.26%) ⬇️
d2q9_bc 23.20% <2.32%> (-0.22%) ⬇️
d2q9_kuper 23.97% <2.32%> (-0.23%) ⬇️
d3q27_PSM_NEBB 31.46% <2.32%> (-0.32%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@llaniewski
Copy link
Member

Before Christmas I started some refactoring, and these things are in a bit of a conflict, so there will be some weird merge-magic going on soon :-)

@kubagalecki If I see a single commit after Jan 1 in your TCLB fork, I'll start harassing you about things you are supposed to be doing.

@llaniewski llaniewski merged commit 05eb7b1 into CFD-GO:unstable Jan 3, 2024
45 of 47 checks passed
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