Skip to content
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

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

stubbiali
Copy link

  • Adapt the Loki config files.
  • Use loki_transform() rather than loki_transform_convert() in CMakeLists.
  • Enable single precision builds.
  • Fix routine VALIDATE_TAYLOR_TEST.
  • Add arch files for MeluXina and LUMI.

Copy link

@reuterbal reuterbal left a 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.

Copy link
Collaborator

@mlange05 mlange05 left a 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
Copy link
Collaborator

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?

Copy link

@reuterbal reuterbal left a 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.

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.

3 participants