-
Notifications
You must be signed in to change notification settings - Fork 283
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
enhance LAMMPS easyblock dynamically add ARMV81
and A64FX
to Kokkos CPU mapping based on LAMMPS version + fix installation of Python bindings for LAMMPS >= 2Aug2023 + fix sanity check by doing MPI_Finalize
#3036
Conversation
easybuild/easyblocks/l/lammps.py
Outdated
@@ -118,6 +118,8 @@ | |||
'zen2': 'ZEN2', | |||
'zen3': 'ZEN3', | |||
'power9le': 'POWER9', | |||
'neoverse_n1': 'ARMV81', | |||
'neoverse_v1': 'ARMV81', |
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.
@laraPPr Did you check whether more recent LAMMPS versions supported better targets for neoverse_v1
?
If so, we can refine this for more recent LAMMPS versions (or use a better target by default, and fall back to ARMV81
for older LAMMPS versions).
Should we also include A64FX
here (which archspec
should recognize as such by producing a64fx
, see https://github.com/archspec/archspec-json/blob/d844bb36b21dfb9ff5d04727edfc08f592fc06af/cpu/microarchitectures.json#L2716
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.
Recent LAMMPS mapping is at https://docs.lammps.org/Build_extras.html#available-architecture-settings
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.
Added the most recent mappings but they do not seem to include better targets for neoverse_v1
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.
We are in a tricky situation here as you also need to update the KOKKOS_CPU_MAPPING
and KOKKOS_GPU_ARCH_TABLE
to match up the new architectures introduced but this has to be done in a backwards compatible way. Probably we need to allow that the values of these dictionaries may be dictionaries themselves with version keys and the appropriate return values.
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.
Should I than test this easyblock with older easyconfigs of LAMMPS to see if anything is broken?
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.
Or update the "constants" based on the LAMMPS version in the easyblock constructor?
Maybe using constants was just wrong here, and it needs to be done via class variables like self.kokkos_cpu_mapping
instead?
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.
We could start doing something like this to update the mapping for particular versions of LAMMPS:
def update_kokkos_cpu_mapping(self):
if LooseVersion(self.cur_version) >= LooseVersion(translate_lammps_version('2Aug2023')):
self.kokkos_cpu_mapping['a64fx'] = 'A64FX'
def __init__(self, *args, **kwargs):
"""LAMMPS easyblock constructor: determine whether we should build with CUDA support enabled."""
super(EB_LAMMPS, self).__init__(*args, **kwargs)
self.kokkos_cpu_mapping = copy.deepcopy(KOKKOS_CPU_MAPPING)
self.update_kokkos_cpu_mapping()
and keep KOKKOS_CPU_MAPPING
as is, assuming that it's correct for all LAMMPS versions we care about
ARMV81
is supported since stable_29Oct2020
it seems, see lammps/lammps@60864e3
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.
ARMv80
, ARMv81
, ARMv8-ThunderX
are actually supported since stable_31Mar2017
, see lammps/lammps@a9f0b7d
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.
ARMv8-TX2
is available since stable_29sep2021
(see lammps/lammps@39786b1) and A64FX
is available since stable_29Sep2021
(see lammps/lammps@eea14c5)
easybuild/easyblocks/l/lammps.py
Outdated
@@ -118,6 +118,8 @@ | |||
'zen2': 'ZEN2', | |||
'zen3': 'ZEN3', | |||
'power9le': 'POWER9', | |||
'neoverse_n1': 'ARMV81', | |||
'neoverse_v1': 'ARMV81', |
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.
We are in a tricky situation here as you also need to update the KOKKOS_CPU_MAPPING
and KOKKOS_GPU_ARCH_TABLE
to match up the new architectures introduced but this has to be done in a backwards compatible way. Probably we need to allow that the values of these dictionaries may be dictionaries themselves with version keys and the appropriate return values.
… scheme2Aug2023_update2
Test report by @branfosj Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (1 easyconfigs in total) |
Test report by @laraPPr Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
Test report by @branfosj Overview of tested easyconfigs (in order)
Build succeeded for 5 out of 5 (1 easyconfigs in total) |
Test report by @branfosj Overview of tested easyconfigs (in order)
Build succeeded for 0 out of 1 (1 easyconfigs in total) |
Test report by @branfosj Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
Test report by @branfosj Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
Test report by @laraPPr Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
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.
LGTM, some minor edits that don't affect any of the testing to date
Co-authored-by: ocaisa <[email protected]>
Co-authored-by: ocaisa <[email protected]>
Co-authored-by: ocaisa <[email protected]>
Just tested on ARM but it failed see: |
The arch detection was done correctly. So the changes made to the easybuild are not causing the failure |
LAMMPS now build correctly on ARM and x86_64 in EESSI: |
Test report by @laraPPr Overview of tested easyconfigs (in order)
Build succeeded for 4 out of 4 (4 easyconfigs in total) |
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.
LGTM
This took a while, thanks for your patience @laraPPr ! |
ARMV81
and A64FX
to Kokkos CPU mapping based on LAMMPS version + fix installation of Python bindings for LAMMPS >= 2Aug2023ARMV81
and A64FX
to Kokkos CPU mapping based on LAMMPS version + fix installation of Python bindings for LAMMPS >= 2Aug2023 + fix sanity check by doing MPI_Finalize
No description provided.