-
Notifications
You must be signed in to change notification settings - Fork 305
enhance LAMMPS easyblock to make it possible to overide kokkos_arch for GENERIC ARM target
#3943
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
Conversation
Signed-off-by: laraPPr <[email protected]>
ocaisa
left a comment
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.
Verified via EESSI/software-layer-scripts#88 (comment)
| if kokkos_arch: | ||
| # If someone is trying a manual override for this case, let them | ||
| if kokkos_arch not in KOKKOS_CPU_ARCH_LIST: | ||
| warning_msg = "Specified CPU ARCH (%s) " % kokkos_arch | ||
| warning_msg += "was not found in listed options [%s]." % KOKKOS_CPU_ARCH_LIST | ||
| warning_msg += "Still might work though." | ||
| print_warning(warning_msg) | ||
| processor_arch = kokkos_arch | ||
| else: | ||
| processor_arch = 'ARMV80' |
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 understand the idea, but this looks weird nonetheless.
This will still print a warning because kokkos_arch is set, even though it is not ignored but will be used. It's also confusing that this only applies for aarch64, even though one might want to use the same approach for x86 when the generic optarch is set.
Instead, we should probably check kokkos_arch first and the optarch if kokkos_arch is not set.
This would allow EESSI to use the overwrite, and increase flexibility in general.
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.
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.
Ah, already done in:
kokkos_arch for GENERIC ARM target
Some context for this change: there is a problem with CUDA<13.1 for Arm architectures which try to leverage NEON instructions. The workaround in EESSI/software-layer-scripts#87 is to override the architecture detection so these are not included. This change allows you to do that even for the generic case.