Exawind / amr-wind

AMReX-based structured wind solver
https://exawind.github.io/amr-wind
Other
105 stars 83 forks source link

Diagnostics bug fix + de-hardcoding refRatio in makeFineMask calls #1050

Closed mbkuhn closed 4 months ago

mbkuhn commented 4 months ago

Summary

Fixing the use of the mask array in PrintMaxMACVelLocations to avoid going out-of-bounds Correcting places where the refinement ratio was hard-coded in makeFineMask calls

Pull request type

Checklist

The following is included:

This PR was tested by running:

Issue Number: #941 #1008

mbkuhn commented 4 months ago

Other details: I added the unit test (commit 406c00c), but it still passed without the bug fix. However, in debug mode, the test doesn't pass because it can properly detect the out-of-bounds index. Is that good enough? Any suggestions?

With the full PR, the bug is fixed, and it does pass the test in debug.

marchdf commented 4 months ago

I think it's fine. We only test OOB in debug mode. So the real thing is when we turn on the nightly test harness we have debug and release runs. Which we used to do and we will do again ;)

mbkuhn commented 4 months ago

This PR does change a lot of different files, but it's not well suited to being tested with regression tests, because the changes are purely in error measurements going to text or netCDF outputs, not in anything that would change the values of the plt files. However, I think the fact that the regression test run fine and the code compiles fine are good indications that those widespread changes are nothing to worry about. The one bug fix that does change the functionality is addressed with a unit test.