- removed comment
ML_BSSN::LapseACoeff and ML_BSSN::ShiftBCoeff Removed without Proper Deprecation
After pulling the latest ET, it appears that practically all of my parameter files no longer work. The reason is simple:
ML_BSSN::LapseACoeff and ML_BSSN::ShiftBCoeff were removed in the McLachlan rewrite and not properly deprecated like all other renamed parameters.
I found this because it broke the IllinoisGRMHD test suite. I am surprised and worried by my finding, because it appears that no other test suite parameter files set these ML_BSSN gauge parameters, so this was not caught earlier.
Perhaps to prevent this from happening in the future, perhaps in some subset of our tests we should manually set all parameters in the test parfiles to their defaults. This way any bug in renaming parameters will be caught immediately.
Keyword: McLachlan
Comments (7)
-
reporter -
reporter - removed comment
Perhaps to prevent this from happening in the future, perhaps in some subset of our tests we should manually set all parameters in the test parfiles to their defaults. This way any bug in renaming parameters will be caught immediately.
I should say, all, otherwise unset parameters should be manually set to their defaults. This should have no impact on the tests, and in addition to catching renamed parameters, it will also, in some cases, catch when parameters are changed from their defaults.
-
- changed status to open
- assigned issue to
- removed comment
-
- removed comment
This should now be corrected in McLachlan. Please test.
-
- changed status to open
- removed comment
-
- removed comment
Zach, does this work for you now?
-
- changed status to resolved
- removed comment
No, it didn't work. evolveA was not steerable, so the compatibility code in ML_BSSN_Helper was not able to steer it. I have fixed McLachlan and pushed the fix. The current WVUThorns test passes when these two parameters are put back to their old names.
- Log in to comment