Skip to content

[WIP] Refactor Command to Avoid Checking for Help and Version #822

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

Draft
wants to merge 21 commits into
base: develop
Choose a base branch
from

Conversation

michaelmckinsey1
Copy link
Collaborator

@michaelmckinsey1 michaelmckinsey1 commented Jun 10, 2025

Description

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

  • Add/modify systems/system_name/system.py file
  • Add/modify a dry run unit test for system_name in .github/workflows/run.yml
  • Add/modify systems/all_hardware_descriptions/hardware_name/hardware_description.yaml which will appear in the docs catalogue

Adding/modifying a benchmark (docs: Adding a Benchmark)

  • If modifying the source code of a benchmark: create, self-assign, and link here a follow up issue with a link to the PR in the benchmark repo.
  • If package.py upstreamed to Spack is insufficient, add/modify repo/benchmark_name/package.py plus: create, self-assign, and link here a follow up issue with a link to the PR in the Spack repo.
  • If application.py upstreamed to Ramble is insufficient, add/modify repo/benchmark_name/application.py plus: create, self-assign, and link here a follow up issue with a link to the PR in the Ramble repo.
  • Tags in Ramble's application.py or in repo/benchmark_name/application.py will appear in the docs catalogue
  • Add/modify an experiments/benchmark_name/experiment.py to define a single node and multi-node experiments
  • Add/modify a dry run unit test in .github/workflows/run.yml

Adding/modifying core functionality, CI, or documentation:

  • Update docs
  • Update .github/workflows and .gitlab/tests unit tests (if needed)

@github-actions github-actions bot added the feature New feature or request label Jun 10, 2025
@michaelmckinsey1 michaelmckinsey1 self-assigned this Jun 10, 2025
@ilumsden
Copy link
Collaborator

I wanted to just add a few options that @michaelmckinsey1 and I discussed relating to this. The 3 options we discussed (and the 3 most viable options besides what @michaelmckinsey1 has already done and without getting into some really complex and esoteric stuff IMO) are:

  1. Split the setup_parser functions for each command into a separate file(s) so that Benchpark imports that are not needed for setting up argparse are not imported. Then, dynamically load only the command function that we need for the selected subcommand in main.py
  2. Require command developers to put all their Benchpark imports into the command function
  3. Add a decorator similar to the example below that will auto import symbols and/or modules and inject them into the decorated function. Although I worked out the example out of curiosity, I really would NOT recommend this approach
from functools import wraps
from importlib import import_module

# Each path string takes the form of "module[:sym0[,sym1[,sym2...]]]"
def benchpark_imports(*import_path_strs):
    def inner_decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            # Loop over the path strings provided to the decorator
            for path_str in import_path_strs:
                # Split the path string into the module name and symbols to import (if provided)
                module_name_and_imports = path_str.split(":")
               # If both module name and symbols to import were provided, ...
                if len(module_name_and_imports) == 2:
                    # Import the module with importlib
                    mod = import_module(module_name_and_imports[0])
                    # Split the symbols to import into individual names
                    syms_to_import = module_name_and_imports[1].split(",")
                    # For each symbol to import, get the symbol from the loaded module.
                    # Then, save it into the decorated function's __builtins__.
                    # This entire process is equivalent to running the following inside the decorated function:
                    # >> from <module_name> import <sym0>, <sym1>, <sym2>, ...
                    for sym in syms_to_import:
                        func.__builtins__[sym] = getattr(mod, sym)
                # Else, if only the module name was provided, ...
                elif len(module_name_and_imports) == 1:
                    # Use built_mod to iteratively build the module name up
                    built_mod = []
                    # Split the module name into individual subpackages/submodules
                    for partial_mod in module_name_and_imports[0].split("."):
                        # Build the current module/package name to import
                        built_mod.append(partial_mod)
                        curr_mod = ".".join("built_mod)
                        # Import the current module/package
                        pmod = import_module(curr_mod)
                        # Save the current module/package into the decorated function's __builtins__.
                        # This entire process is equivalent to something like:
                        # >> import os
                        # >> import os.path
                        func.__builtins__[curr_mod] = pmod
                # Otherwise, the user input is invalid
                else:
                    raise ValueError(
                        "Invalid path/import specifier: {}".format(path_str)
                    )
            # Call the actual decorated function
            return func(*args, **kwargs)

        return wrapper

    return inner_decorator

# Using the decorator here is equivalent to putting the following
# inside of the main function:
# >> from test_print import test
@benchpark_imports("test_print:test")
def main():
    print("Hit main")
    test()

@michaelmckinsey1 michaelmckinsey1 changed the title [WIP] Refactor cmd [WIP] Refactor Command to Avoid Checking for Help and Version Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants