cburstedde / p4est

The "p4est" forest-of-octrees library
www.p4est.org/
GNU General Public License v2.0
261 stars 115 forks source link

Checksum comparison in wrap_adapt #289

Closed hannesbrandt closed 8 months ago

hannesbrandt commented 9 months ago

We add a checksum comparison in wrap_adapt to check whether coarsening and balancing cancel each other out.

One checksum is computed at the start of the function for the p4est of the input wrap. After refining, coarsening and balancing we compute another checksum, if the total amount of quadrants did not change. If the result p4est has the same checksum as the input p4est, we do not compute a new ghost and mesh for the wrap.

cburstedde commented 8 months ago

Thanks; I believe I am missing some updates in the p8est*.h files.

There is the suggestion to remove checksum_partition, can you recheck with @tim-griesbach if that function is (still?) needed?

tim-griesbach commented 8 months ago

There is the suggestion to remove checksum_partition, can you recheck with @tim-griesbach if that function is (stil?) needed?

@hannesbrandt and I talked about removing p4est_checksum_partition. Since it is used in test_valid2.c and may be useful for testing potential future changes to the partition implementation, we think it may beneficial to keep the function.

cburstedde commented 8 months ago

Would we be able to finalize this PR? Formally the matching changes to p8est_communication.h would still be required.

Once p4est_checksum is collective, would it make sense to go through our examples/tests and align them to the new behavior?

hannesbrandt commented 8 months ago

Once p4est_checksum is collective, would it make sense to go through our examples/tests and align them to the new behavior?

I have gone through the tests and adapted test_io2.c . The other tests and examples only compare checksums of different timesteps and did not need to be adapted.

Additionally, I switched the data types in comm_checksum from uint64_t to long long, since this is the format they are sent in.

cburstedde commented 8 months ago

Would you think we're good to go on this one?

hannesbrandt commented 8 months ago

Would you think we're good to go on this one?

I would like to address one last issue that I just noticed. Calling p4est_checksum aborts if zlib is not available. Should I adapt the code to only compute the checksum in wrap_adapt, if zlib is available?

cburstedde commented 8 months ago

Would you think we're good to go on this one?

I would like to address one last issue that I just noticed. Calling p4est_checksum aborts if zlib is not available. Should I adapt the code to only compute the checksum in wrap_adapt, if zlib is available?

Yes that sounds good. If you could document this in the .h files?

cburstedde commented 8 months ago

Thanks!