NanException, plus some new small fixes and features

[benmagruder]

The biggest change in this PR is the addition of the NanException class in pscf/iterator/, which is used in a try-catch block of AmIteratorTmpl.tpp to catch the appearance of a NaN value in the AmIterator residual without crashing the whole program. This has been briefly tested on some examples that are known to generate NaN values in the residual, and the new code works as-designed. However, some implementations of the g++ compiler will basically ignore NaN values because of the -ffast-math tag, which prevents NanExceptions from being thrown in the first place. This is an unaddressed issue in the current pull request, and will be addressed later. Additionally, I have not yet figured out how to incorporate this NanException into pspg::AmIteratorGrid, which performs all of its arithmetic on the GPU, so that should be the subject of a future update as well.

Other changes include edits to the Sweep param file block, which now contains optional writeCRGrid, writeCBasis, and writeWRGrid inputs to control which fields are output at each sweep step. The System::writeThermo method (in pspg and pspc) now outputs the lattice parameters for any calculation where isFlexible=true.

Finally, some documentation was updated to address certain aspects of the thin film feature, such as its inability to be paired with the COMPUTE command and its poor performance when paired with the reuseState feature.

All unit tests have been run, and they pass successfully.

[benmagruder]

Thanks for the comments, I agree and will make the changes. I have one additional question about your proposal for allowing user inputs to control the behavior of the symmetry check inside of convertKGridToBasis and convertRGridToBasis: do you have any thoughts about where the user can/should be able to input their preferred epsilon value, and how they can indicate whether or not they want to check the symmetry? It will have to be in the param file or the command file somewhere, but I can't think of a good logical place for that input to be added. Maybe just as an optional third parameter in the command file, e.g. "RGRID_TO_BASIS c.rf c.bf 1e-8"?

[dmorse]

Ben, I thought it was good design to leave a programmer the option of controlling tests at the level of the function interface. This was partly to leave flexibility to obtain different behavior in different contexts, such as their use within the iteration algorithm vs. use in a conversion command. Given that flexibility, I might choose turn off those tests within the iteration loop, when time of the test could conceivably matter and would have good reason to believe that the fields all remain symmetric to within round off error. I also think, however, that it would introduce too much clutter into the user interface to give the user control over this in the command or parameter file. I recommend that the (WHATEVER)_TO_BASIS commands should call a version of the function that applies a test with some default choice of epsilon, since the computational time to do the test is irrelevant in this context an inexperienced user might indeed try to convert a non-symmetric field to a symmetry-adapted basis format. If we want to give the user the ability to explicitly check the symmetry from the command file with a specified threshold, the way to do that is to introduce a required parameter epsilon for the command CHECK_RGRID_SYMMETRY. Unfortunately, the command file format wasn't designed to allow for optional command parameters, so we would have to make it a required parameter, but I think that would be a good idea. That would provide a way for a more sophisticated user to check if the field is symmetric to within whatever threshold they want whenever they think is appropriate. Once a user has satisfied themselves that a field is symmetric to within a specified threshhold, they call the conversion function with confidence.

On Thu, Jan 5, 2023 at 9:41 PM Ben Magruder @.***> wrote:

Thanks for the comments, I agree and will make the changes. I have one additional question about your proposal for allowing user inputs to control the behavior of the symmetry check inside of convertKGridToBasis and convertRGridToBasis: do you have any thoughts about where the user can/should be able to input their preferred epsilon value, and how they can indicate whether or not they want to check the symmetry? It will have to be in the param file or the command file somewhere, but I can't think of a good logical place for that input to be added. Maybe just as an optional third parameter in the command file, e.g. "RGRID_TO_BASIS c.rf c.bf 1e-8"?

— Reply to this email directly, view it on GitHub https://github.com/dmorse/pscfpp/pull/103#issuecomment-1373104206, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOB4IEE3DRJWQNU66TSNZDWQ6H5BANCNFSM6AAAAAATSPR644 . You are receiving this because you commented.Message ID: @.***>

[benmagruder]

Everything should be up and running now and all comments above have been addressed, and I ran a few example cases to make sure they run properly. However, I hadn't yet run/tweaked the unit tests so a couple of those might still need adjustments to accommodate the new changes.

[dmorse]

I've merged your recent changes into my feature pull branch already, but will wait for a commit statement or message from you saying that all unit tests have been checked before finishing the merge and merging into my devel.

[benmagruder]

The unit tests all run and pass successfully with 1 small tweak: in pspc/tests/SystemTest.h, lines 152 and 232 need to be edited because System::checkRGridFieldSymmetry now requires an additional input, epsilon. So I added 1.0e-8 (a good default value) as the second parameter passed to checkRGridFieldSymmetry and all unit tests pass (on both MSI and my personal laptop). It's probably easiest if you just make the edit on your end rather than open a whole new pull request, but I can open a PR if you want.

Alternatively, we could give checkRGridFieldSymmetry a default value for epsilon, but the only time that function is used is when we call CHECK_RGRID_SYMMETRY in the command file, so I chose to make epsilon required because it is now a required parameter for that command.