-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Miscellaneous of updates and adaptations #13
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look great, many thanks!
I haven't run it, yet, hence only a comment so far, but visually everything seems ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for these changes and the cleanup @stubbiali . I can confirm the Meluxina arch works as expected, and the other bug fixes and clean-ups are also very nice.
The only thing I don't think should be in this PR is the switch to single precision, as this requires separate validation and a specific mention in the README etc. With a quick trial on our host nodes I already got this to not validate (Intel) or fail (Nvidia), and since CLOUDSC2 was specifically release for TL/AD exploration I'm reluctant to enable this without further investigation of results. Could you please remove this change, and file this as a separate PR/issue for a dedicated discussion?
Once this is addressed, and @reuterbal confirms the LUMI setup I think we're good to go here.
ecbuild_find_package( NAME loki ) | ||
|
||
# Add option for single-precision builds | ||
ecbuild_add_option( FEATURE SINGLE_PRECISION | ||
DESCRIPTION "Build CLOUDSC in single precision" DEFAULT OFF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be packed in with a general housekeeping PR, as single-precision is effectively not working for the Fortran variants. In my quick and easy experimentation I got floating point errors with Nvidia compiler on host side. Would you mind taking this to a separate PR for proper due diligence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested the 15.0.1 arch on LUMI. Building works fine, apart from ad/tl plain SCC variants, which crash the compiler (what we have seen with CLOUDSC as well).
NL runs fine, too. However, tl/ad crash with a segfault on-device for me, suggesting something is not quite right there. This requires some further investigation but is out of scope for this PR.
loki_transform()
rather thanloki_transform_convert()
in CMakeLists.VALIDATE_TAYLOR_TEST
.