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

Adaptive Hole Map #12

Closed
wants to merge 17 commits into from
Closed

Conversation

ackirby88
Copy link
Contributor

@ackirby88 ackirby88 commented Jan 19, 2023

Initial implementation of the adaptive hole map algorithm.
To enable/disable this functionality, set line 64 in codetypes.h

#define USE_ADAPTIVE_HOLEMAP 1 // [0] original hole map, [1] adaptive hole map

I've done preliminary testing on the tioga test case: unstructured tetrahedral sphere and Cartesian background mesh.

Test Case Input (case/grid/input):

 $inputs
 nlayers=40,
 wallSpacing=1e-5,
 outerSpacing=0.1,
 outerB=0.05,
 xlo(1)=-5.,
 xlo(2)=-5.,
 xlo(3)=-5.,
 xhi(1)=5.,
 xhi(2)=5.,
 xhi(3)=5.,
 dx(1)=0.1,
 dx(2)=0.1,
 dx(3)=0.1,
 $end

Original Hole Map (./run.sh 12):

 Generating           24  partitions
 Finished generating grid ..
 Writing tecplot compatible files..
 All done
 # tioga test on           12  processes
 # tioga test : finished reading grids
 connectivity time=  0.71787999999999996     
           2          14
           0           0
           0           0
           1           7
           0           0
           0           0
           2          14
           0           0
           0           0
           3          21
           0           0
           0           0
           5          35
           0           0
           0           0
           0           0
           0           0
           2          14
           0           0
           0           0
           0           0
           0           0
        1202       10818
        1282       11538
 data update time=   7.0409999999998529E-003
-- Interpolation error statistics --
ProcId    Error
   0     0.6619866E-17
   3     0.5463998E-17
   4     0.6485183E-17
   5     0.4214754E-17
   6     0.6526902E-17
   7     0.3255305E-16
   8     0.7567119E-17
   9     0.8691998E-17
  10     0.3855176E-17
  11     0.6595807E-17
   1     0.9389718E-17
   2     0.7459847E-17
myid/meshtag/norphan/nnodes/nobc=4 2 1150 85000 0
myid/meshtag/norphan/nnodes/nobc=7 2 1138 85000 0
Tioga exited successfully

Adaptive Hole Map (./run.sh 12):

 Generating           24  partitions
 Finished generating grid ..
 Writing tecplot compatible files..
 All done
 # tioga test on           12  processes
 # tioga test : finished reading grids
  [tioga::performConnectivity::getAdaptiveHoleMap] Mesh Body 0: Levels 8, total octants 34353, total leafs 30059
 connectivity time=   9.9077000000000026E-002
           1           7
           0           0
           0           0
           0           0
           0           0
           2          14
           0           0
           0           0
           3          21
           0           0
           0           0
           5          35
           0           0
           0           0
           2          14
           0           0
           0           0
           2          14
           0           0
           0           0
           0           0
        1202       10818
        1282       11538
           0           0
 data update time=   4.9900000000000500E-003
-- Interpolation error statistics --
ProcId    Error
   0     0.6619866E-17
   2     0.7459847E-17
   4     0.6485183E-17
   6     0.6526902E-17
   7     0.3255305E-16
   9     0.8691998E-17
   1     0.9389718E-17
   3     0.5463998E-17
   5     0.4214754E-17
   8     0.7567119E-17
  10     0.3855176E-17
  11     0.6595807E-17
myid/meshtag/norphan/nnodes/nobc=7 2 1055 85000 0
myid/meshtag/norphan/nnodes/nobc=4 2 1058 85000 0
Tioga exited successfully

AdaptiveHoleMap sphere 8 levels

@michaeljbrazell
Copy link

nice, way faster!

…with an outer/overset bc face and wall bc face -- this prevents the adaptive hole map from refining to the max number of levels. (2) Fix compiler warnings about the intstring character array buffer size.
…memory cleanup. Note: suspect small memory leak in exchangeBoxes().
@jrood-nrel
Copy link
Collaborator

Can there be something other than a define to turn this on? Also, can we verify this has been run through ASAN or valgrind to see that there are no leaks?

…compiler define variable). Also added memory cleanup for tioga shutdown -- checked with valgrind (iblank_cell is lost but only because it is handled by nalu-wind).
@ackirby88
Copy link
Contributor Author

Hi Jon,
I added a new tioga interface function to set the hole map algorithm:

void tioga_setholemapalg_(int *alg); // [0] original hole map, [1] adaptive hole map

This function replaces the compile-time define variable.

Second, I added some memory cleanup for tioga instance deletion.
There were no issues previously for repeated hole map constructions, but only when tioga_delete was called.
I verifed no memory leaks using Valgrind (single rank and 2 mpi ranks -- output files attached below).

valgrid.adaptHoleMap.1rank.txt
valgrid.adaptHoleMap.2ranks.txt

verbose=0;
if(iblank[i] < 0){
temp=donorList[i];
while(temp!=NULL){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to exist?

Copy link
Contributor Author

@ackirby88 ackirby88 Feb 8, 2023

Choose a reason for hiding this comment

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

It's a hold-over from the original function lines 320-330, so I wouldn't change the functionality.
But it does look potentially buggy with the while construct. Let me track down the history of the donorList.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering if there's a bug in the original function. My guess is there should be
'''temp=temp->next;'''
Inside the while loop. Thoughts ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it doesn't make any sense without some kind of increment inside the while

Copy link
Contributor

Choose a reason for hiding this comment

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

Does indeed seem suspect ... Do you see a reason why it hasn't broken anything yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it worked before because Jay already sorted the temp donor resolutions (finest donor first) inside insertInList and already established the iblank value to be a fringe (-1). Otherwise, it would have a better resolution than the donor and not entered that if-statement check.

@ashesh2512
Copy link
Contributor

@ackirby88 Where is the .svn folder coming from?

@ackirby88
Copy link
Contributor Author

ackirby88 commented Apr 2, 2023

Where is the .svn folder coming from?

@ashesh2512 It was from the Jay's original TIOGA repository.

@ackirby88
Copy link
Contributor Author

@ashesh2512 I don't see your commits here...

verbose=0;
if(iblank[i] < 0){
temp=donorList[i];
while(temp!=NULL){
Copy link
Contributor

Choose a reason for hiding this comment

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

Does indeed seem suspect ... Do you see a reason why it hasn't broken anything yet?

driver/CMakeLists.txt Outdated Show resolved Hide resolved
driver/CMakeLists.txt Outdated Show resolved Hide resolved
src/codetypes.h Show resolved Hide resolved
src/codetypes.h Show resolved Hide resolved
src/MeshBlock.h Show resolved Hide resolved
src/MeshBlock.h Outdated
int nrank;
int masterID; /* master rank for distributing mesh block data */
MPI_Comm comm; /* communicator containing all complement ranks + master */
MPI_Request req;
Copy link
Contributor

Choose a reason for hiding this comment

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

is req used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so -- I originally had asynchronous communications.
I'll check to see if I use it for the abutting mesh implementation (next week); if not, I'll remove it.

src/tiogaInterface.C Outdated Show resolved Hide resolved
src/MeshBlock.C Outdated Show resolved Hide resolved
src/MeshBlock.C Outdated Show resolved Hide resolved
@ashesh2512
Copy link
Contributor

@ashesh2512 I don't see your commits here...

@ackirby88 Turns out I never submitted the comments. You should be able to see them now.

…ap specific functions (comms), (2) macro replacement, (3) increase max level to all full 32-bit usage, (4) add back poisson_mms.
@ackirby88
Copy link
Contributor Author

@ashesh2512 I committed your recommended changes. Please give the built-in test case a try along with your other tests.

src/tioga.h Outdated Show resolved Hide resolved
@ackirby88
Copy link
Contributor Author

@jrood-nrel @ashesh2512 I've merged in the latest exawind changes with the std::min/max fixes. I've also addressed your suggestions, Jon, about the malloc.h.
This may finally be a good time from NREL's side to complete this pull request and merge in these changes so we stop diverging.

@ashesh2512
Copy link
Contributor

Merged through #14

@ashesh2512 ashesh2512 closed this Aug 1, 2023
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.

5 participants