FIX: runtime errors when compiled with flang #193
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Issue was flang picking up some other version of ACOSD.
|
This uncovered a bug in flang! There is an open PR upstream to fix it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the run-time issue when complied with flang on windows noted in #135 (comment)
The first commit is adding explicit declarations of variables which seems like good practice (but is unrelated to the problem).
The second commit fixes the problem we identified with incorrect results when complied with flang.
The third commit fixes structurally similar issues that we have not disocvered as bugs yet (presumably because they would have only shown up on windows when compiled with flang which we otherwise abandoned work on due to the first issue!)
The root of the problem is that with both gfortran and flang the implemenattion of ACOSD in the source was not actually being used (I had a version of this with lots of print statements and without the EXTERNAL lines the ACOSD logs did not come out). It seems that there is a bug in flang's ACOSD intrinsic (?! 🤯 I could not sort out exactly what it was doing as wrong result was not just the radian version of the right result). By adding the EXTERNAL flag the linker picks the version in the source which works.
I could also see eliminating ACOSD (and friends) all together and working in radians in the code, but that would be a bigger (and riskier) change. I could also see removing the local local version of the degree sub-routines and sorting out which versions of flang they work correctly in, but that is a bigger project than I want to take on right now.
It seems in the last year CF has started to provide gfortran so this is a bit moot (sorry @briantoby , I incorrectly assumed we had moved to pulling gfortran for windows from the defaults channel), and my attempts to test with this branch on windows with flang failed with compiler errors (I think it is picking the wrong version of flang that does not play nice with visual studio). The pixi.lock file in https://github.com/tacaswell/GSAS-II/tree/clang-works compiles and is the branch I used to verify that this fixes the problem.