Skip to content

Conversation

@Maxwell-Rosen
Copy link
Collaborator

@Maxwell-Rosen Maxwell-Rosen commented Dec 29, 2025

Bug fix

Summary

Description:

Mana informed me that the twist-shift BC unit test was failing and it seemed to be caused by the position map.
image
image

I found this quite strange because when I ran these unit tests on perlmutter GPU, they were just fine on main.
image

In any case, I did see a potential issue with memory handling regarding how the gk_geometry modules create a null position map if none is provided. This memory handling could lead to different behavior on different machines (although here, both Mana and I were testing on Perlmutter GPU). Even though I can't reproduce the same failure, I cleaned up the code by intentionally allocating a null position map in the unit tests and passing that to the geometry generators.

Solution

Create a null position map in each unit tests.
Remove code in the gk_geometry generators to detect if no position map is passed.
Removed unused "flags" variable from the position map.

Automated testing: unit tests are updated. position_map tests are updated accordingly.

Community Standards

  • Documentation has been updated.
  • My code follows the project's coding guidelines.
  • Changes to layer/zero should have a unit test, e.g., core/zero.

Testing:

  • I added a regression test to test this feature.
  • I added this feature to an existing regression test.
  • I added a unit test for this feature.
  • Ran make check and unit tests all pass.
  • I ran the code with make valcheck, and it is clean.
  • I ran the code through computer-sanitizer on GPU, and it is clean.
  • I ran a few regression tests to ensure no apparent errors.
  • Tested and works on CPU.
  • Tested and works on multi-CPU.
  • Tested and works on GPU.
  • Tested and works on multi-GPU.

Additional Notes

I would like @manauref to run the appropriate tests before this is merged. I am unable to reproduce the original error, so I cannot verify that these modifications fix the issue.

… and have every unit test create a null position map
…ing in ctest_gk_geometry_tok which needed to be updated after the filepath of the .geqdsk was moved in a recent PR. It's really difficult to tell if I broke something in this branch because so many unit tests are failing that the errors exceed my terminal context length. I will update the issue about failing unit tests. It's very important that our unit tests pass so that we can have reliable checks that we didn't break anything. It would be really great to have nightly reminders about any unit tests which are broken on main. Also, it's really anoying that when some unit tests fail, it spits out like 10 thousand lines of failures instead of just one line that the unit test failed.
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.

2 participants