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.

Sign up for GitHub

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

includes: name mangle ramble and include from spack normally #469

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

includes: name mangle ramble and include from spack normally #469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4c4155f
name mangle ramble spack files on bootstrap
becker33 Dec 3, 2024
23de37b
spack imports: hacky solution no longer needed
becker33 Dec 3, 2024
2b25c2c
setup: use llnl.util.link_tree instead of reimplementing
becker33 Dec 3, 2024
f5b3522
lint
becker33 Dec 3, 2024
65e9100
cmd/init: command to initialize one system and any number of experiments
becker33 Oct 9, 2024
a49e3c5
Revert "setup: use llnl.util.link_tree instead of reimplementing"
becker33 Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions lib/benchpark/cmd/init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# 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

import argparse
import os
import shutil
import sys

import benchpark.experiment
import benchpark.spec


def init(args):
# Handle conjoined arguments that argparse doesn't separate
specs_str = " ".join(args.specs)
experiment_args, system_args = specs_str.split("--")

# Parse and concretize system
system_spec = benchpark.spec.SystemSpec(system_args).concretize()
system = system_spec.system

# Parse and concretize experiments
exp_spec_parser = benchpark.spec.SpecParser(
benchpark.spec.ExperimentSpec, experiment_args
)
experiment_specs = [es.concretize() for es in exp_spec_parser.all_specs()]
experiments = [es.experiment for es in experiment_specs]

# Create system directory and print out yaml
sysdir = os.path.join(args.basedir, system.system_uid())

try:
os.mkdir(sysdir)
system.generate_description(sysdir)
except FileExistsError:
print(f"Abort: system dir already exists ({sysdir})")
sys.exit(1)
except Exception:
# If there was a failure, remove any partially-generated resources
shutil.rmtree(sysdir)
raise

# For each experiment, create experiment directory and print out yaml
for experiment_spec, experiment in zip(experiment_specs, experiments):
expdir = os.path.join(args.basedir, str(hash(experiment_spec)))

try:
os.mkdir(expdir)
experiment.write_ramble_dict(f"{expdir}/ramble.yaml")
except FileExistsError:
print(f"Abort: workload dir already exists ({expdir})")
sys.exit(1)
except Exception:
# If there was a failure, remove any partially-generated resources
shutil.rmtree(expdir)
raise


def setup_parser(parser):
parser.add_argument(
"--basedir",
required=True,
help="Generate a system dir under this, and place all files there",
)

parser.add_argument(
"specs",
nargs=argparse.REMAINDER,
metavar="experiment_spec(s) -- system_spec",
help="Experiment spec(s) and system spec",
)


def command(args):
init(args)
42 changes: 37 additions & 5 deletions lib/benchpark/runtime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import os
import pathlib
import shlex
import stat
import subprocess
import sys

Expand Down Expand Up @@ -82,21 +83,52 @@ def __init__(self, dest):
self.spack_location = self.dest / "spack"

def bootstrap(self):
# Bootstrap Spack first because we will use Spack resources while bootstrapping ramble
if not self.spack_location.exists():
self._install_spack()

spack_lib_path = self.spack_location / "lib" / "spack"
externals = str(spack_lib_path / "external")
if externals not in sys.path:
sys.path.insert(1, externals)
internals = str(spack_lib_path)
if internals not in sys.path:
sys.path.insert(1, internals)

if not self.ramble_location.exists():
self._install_ramble()

ramble_lib_path = self.ramble_location / "lib" / "ramble"

# If ramble is present but not yet name mangled, do that
ramble_spack_path = ramble_lib_path / "ramble_spack"
if not ramble_spack_path.exists():
self._mangle_ramble()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think ideally we would conceptually distinguish the case where we clone ramble as a library vs. when we clone it to run it.

  • e.g. _install_ramble can be called from bootstrap() or .ramble() (via ramble_first_time_setup)
  • I'd recommend making the "library-ness" a property of the RuntimeResources object, and move this consideration to _install_ramble()
    • And maybe rename _install_ramble to reflect that it can update existing installations (i.e. so that this works for users that do a git pull of Benchpark with an already-established Ramble lib prefix


externals = str(ramble_lib_path / "external")
if externals not in sys.path:
sys.path.insert(1, externals)
internals = str(ramble_lib_path)
if internals not in sys.path:
sys.path.insert(1, internals)

# Spack does not go in sys.path, but we will manually access modules from it
# The reason for this oddity is that spack modules will compete with the internal
# spack modules from ramble
if not self.spack_location.exists():
self._install_spack()
def _mangle_ramble(self):
"""Name mangle ramble spack.* symbols to allow separate spack imports."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a bit of detail: mention we are replacing all instances of spack with ramble_spack, and all instances of llnl with [...].

Might be worth broadly outlining why this is expected to work: I don't think this name mangling would make all parts of Spack work, but I believe they make all the parts of Spack that Ramble uses work.

import llnl.util.filesystem as fs

files = [
f
for f in fs.find(self.ramble_location, "*")
if os.stat(f).st_mode & stat.S_IWUSR and not os.path.isdir(f)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we don't own these files, we should probably fail, vs. silently doing nothing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add a comment -- the main purpose here isn't to catch files we don't own, it's to catch files that aren't writable (like some things within the .git repo).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume nothing under .git should be modified: I recommend skipping it entirely (and explicitly).

]
file_filter = fs.FileFilter(*files)
# Don't replace if it's already replaced or if it's a field in an existing module
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think trying to make these regexes idempotent will make them more confusing than they need to be: the existence of the ramble_spack directory can function as a sentinel (if you are worried about CTRL-C, I'd recommend a marker file); if we depend on that, I think we can stop considering _, which will simplify the regex.

Either way, I think this will work as long as:

# spack always does this
import spack.util.spack_yaml
# and never does this (which is allowed)
from spack.util import spack_yaml

But IMO it would be more straightforward to look for [\s]spack (i.e. spaces followed by spack)

I think including some examples of things you intend not to match would be useful (e.g. ...spack_yaml

file_filter.filter(r"(?<!_|\.)spack\.", "ramble_spack.")
file_filter.filter(r"(?<!_)llnl\.", "ramble_llnl.")

ramble_lib_path = self.ramble_location / "lib" / "ramble"
os.rename(ramble_lib_path / "spack", ramble_lib_path / "ramble_spack")
os.rename(ramble_lib_path / "llnl", ramble_lib_path / "ramble_llnl")

def _install_ramble(self):
print(f"Cloning Ramble to {self.ramble_location}")
Expand Down
34 changes: 3 additions & 31 deletions lib/benchpark/system.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@
# SPDX-License-Identifier: Apache-2.0

import hashlib
import importlib.util
import os
import pathlib
import sys
import yaml

import benchpark.paths
Expand All @@ -25,36 +23,10 @@
import ramble.config as cfg # noqa
import ramble.language.language_helpers # noqa
import ramble.language.shared_language # noqa
import spack.util.spack_yaml as syaml # noqa

# We cannot import this the normal way because it from modern Spack
# and mixing modern Spack modules with ramble modules that depend on
# ancient Spack will cause errors. This module is safe to load as an
# individual because it is not used by Ramble
# The following code block implements the line
# import spack.schema.packages as packages_schema
schemas = {
"spack.schema.packages": f"{bootstrapper.spack_location}/lib/spack/spack/schema/packages.py",
"spack.schema.compilers": f"{bootstrapper.spack_location}/lib/spack/spack/schema/compilers.py",
}


def load_schema(schema_id, schema_path):
schema_spec = importlib.util.spec_from_file_location(schema_id, schema_path)
schema = importlib.util.module_from_spec(schema_spec)
sys.modules[schema_id] = schema
schema_spec.loader.exec_module(schema)
return schema


packages_schema = load_schema(
"spack.schema.packages",
f"{bootstrapper.spack_location}/lib/spack/spack/schema/packages.py",
)
compilers_schema = load_schema(
"spack.schema.compilers",
f"{bootstrapper.spack_location}/lib/spack/spack/schema/compilers.py",
)
import spack.util.spack_yaml as syaml # noqa
import spack.schema.packages as packages_schema # noqa
import spack.schema.compilers as compilers_schema # noqa


_repo_path = benchpark.repo.paths[benchpark.repo.ObjectTypes.systems]
Expand Down
7 changes: 7 additions & 0 deletions lib/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import benchpark.cmd.audit
import benchpark.cmd.system
import benchpark.cmd.experiment
import benchpark.cmd.init
import benchpark.cmd.setup
import benchpark.cmd.unit_test
import benchpark.paths
Expand Down Expand Up @@ -198,6 +199,11 @@ def init_commands(subparsers, actions_dict):
system_parser = subparsers.add_parser("system", help="Initialize a system config")
benchpark.cmd.system.setup_parser(system_parser)

init_parser = subparsers.add_parser(
"init", help="Initialize a set of experiments with a compatible system"
)
benchpark.cmd.init.setup_parser(init_parser)

experiment_parser = subparsers.add_parser(
"experiment", help="Interact with experiments"
)
Expand All @@ -218,6 +224,7 @@ def init_commands(subparsers, actions_dict):
)
benchpark.cmd.audit.setup_parser(audit_parser)

actions_dict["init"] = benchpark.cmd.init.command
actions_dict["system"] = benchpark.cmd.system.command
actions_dict["experiment"] = benchpark.cmd.experiment.command
actions_dict["setup"] = benchpark.cmd.setup.command
Expand Down