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

Refactor Systems to Initialize Using Dictionaries #658

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

Conversation

michaelmckinsey1
Copy link
Collaborator

@michaelmckinsey1 michaelmckinsey1 commented Mar 4, 2025

Refactor the way systems are initialized to be similar to how experiments are initialized, using dictionaries instead of multi-line strings and yaml files.

  • Addresses Use dictionary to write yaml in system.py (similar to experiment.py) #654
  • Write a python utility to read in file1 and file2 into a dictionary (does file1 get deprecated when we start using compilers as packages @becker33 ??)
  • Write docs on how to generate a package list on your system, and convert to a dictionary for defining a new system
  • Write docs on how to refresh a package list on your already defined system
  • How to handle multiple versions of available libraries?
    • list-like
                     "externals": [
                         {"spec": "cmake@3.20.2", "prefix": "/usr"},
                         {"spec": "cmake@3.23.1", "prefix": "/usr/tce"},
                         {"spec": "cmake@3.24.2", "prefix": "/usr/tce"},
                     ],
    
  • Write python utility to write out a dictionary to yaml (does the experiment example write a temporary file in a workspace?)
    • Not necessary as yaml.dump will do this.
  • What is needed in bin to use it this way? Look at the experiment as an example.
    • Nothing additional is needed
  • Update all system.py's which have yaml files stored
  • For packages stored as a string right now, move the string to a file first? Packages stored as strings converted to dictionaries in-place.
  • Create script to validate generated systems are the same as previous -- for the system using yaml files, this may be a yaml to dictionary comparison, which can also be useful for knowing when the package dict on the system needs to be updated.
    image

Adding/modifying a system (docs: Adding a System)

Done (Migrate information in yamls systems/x/*.yaml to system.py)

  • llnl-cluster
  • aws-pcluster
  • csc-lumi
  • cscs-daint
  • cscs-eiger
  • generic-x86
  • jsc-juwels
  • lanl-venado
  • llnl-elcapitan
  • llnl-sierra
  • riken-fugaku

Adding/modifying core functionality, CI, or documentation:

  • Modify lib/benchpark/system/py and lib/benchpark/cmd/system.py
  • Docs on generating a yaml file, converting to a dictionary to store in a system.py, updating

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want these refactored into the system.py, correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. @becker33, what do you think?

@michaelmckinsey1
Copy link
Collaborator Author

I've noticed how this is currently done in develop, some files have headers and some do not:

Case 1

# Copyright 2023 Lawrence Livermore National Security, LLC and other
# Benchpark Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: Apache-2.0
  • {SYSTEM}/auxiliary_software_files/compilers.yaml
  • {SYSTEM}/auxiliary_software_files/packages.yaml

Case 2

# SPDX-License-Identifier: Apache-2.0
  • {SYSTEM}/variables.yaml

Case 3

No headers

  • {SYSTEM}/software.yaml
  • {SYSTEM}/software_id.yaml

Shouldn't this be consistent?

@michaelmckinsey1 michaelmckinsey1 requested a review from pearce8 March 4, 2025 01:30
@michaelmckinsey1 michaelmckinsey1 changed the title [WIP] Refactor System to Initialize Using Dictionaries [WIP] Refactor Systems to Initialize Using Dictionaries Mar 4, 2025
Copy link
Collaborator

@pearce8 pearce8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelmckinsey1 I added more tasks in this PR. Take a look and let me know what is already tackled - vs. needs to be added.

@michaelmckinsey1
Copy link
Collaborator Author

michaelmckinsey1 commented Mar 4, 2025

benchpark/systems/csc-lumi/externals/libsci/02-rocmcc-packages.yaml is apparently not being used, should this be deleted?

@michaelmckinsey1 michaelmckinsey1 force-pushed the refactor/systems-i654 branch 2 times, most recently from ad9407e to e6a1ffa Compare March 5, 2025 20:33
@github-actions github-actions bot added the ci CI, unit tests, GitHub actions label Mar 5, 2025
@michaelmckinsey1 michaelmckinsey1 marked this pull request as ready for review March 6, 2025 00:19
@michaelmckinsey1 michaelmckinsey1 added the system New or modified system config label Mar 6, 2025
@michaelmckinsey1 michaelmckinsey1 changed the title [WIP] Refactor Systems to Initialize Using Dictionaries Refactor Systems to Initialize Using Dictionaries Mar 6, 2025
@michaelmckinsey1 michaelmckinsey1 added the ready for review Ready for review label Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci CI, unit tests, GitHub actions ready for review Ready for review system New or modified system config
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use dictionary to write yaml in system.py (similar to experiment.py)
2 participants