search

ECP-copa / ExaMPM

Material point method proxy application based on Cabana.
BSD 3-Clause "New" or "Revised" License
9 stars 11 forks source link

Adjust index calculation for halo and time step, add CL usage #37

Closed markstock closed 1 year ago

markstock commented 1 year ago

Here are a set of bug fixes, usability improvements, and comment corrections. I wasn't sure whether these should be packaged into multiple PRs or one for simplicity.

The bugs were in calculating the range of indices for the non-halo domain and the application of the BCs vs. cell index. Before, the DamBreak sim would show frozen particles in the range 0.94 < y < 1 (for dx=0.01 and 3 halo cells) and asymmetry in the y axis.

Additionally, the codes would write output at steps 0, 1, 101, ... when setting100 steps between output. Now it writes output at 0, 100, 200, etc.

streeve commented 1 year ago

Thanks for the multi-fixup PR! Looks like just the run comment needs to be reformatted. We have a custom make format if clang-format is found during build. I can format if needed too

markstock commented 1 year ago

It's better if I get into the habit of doing that. Update coming...

streeve commented 1 year ago

I got a chance to test this locally - looks great. The only question I have is whether we want to keep the 1D dam break (as in the comments) or keep the 2D version and update the comment (each are useful and it's pretty easy to change) @sslattery @abisner @kwitaechong any thoughts? Also I'd prefer to squash the format commits - happy to do that

streeve commented 1 year ago

@markstock I thought you had mentioned there were some things you wanted to confirm here - let me know

markstock commented 1 year ago

I wanted to be sure that the changes didn't affect the crash during halo communications, and they DO (seem to suppress it).

And as for the dam break being uniform in the y-direction: replicating 2D results in a 3D code is a typical test that I do to find bugs in both algorithm and implementation. It definitely helped here.

streeve commented 1 year ago

That's great! We're testing this in a few other places now so we'll also let you know if we find any similar issues