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

[experimental] Adding a CudaExperiment, and variables to Base and CudaExperiment #400

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
100 changes: 100 additions & 0 deletions lib/benchpark/cuda.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# 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

from typing import Dict
import yaml # TODO: some way to ensure yaml available

from benchpark.directives import Experiment
import benchpark.spec
import benchpark.paths
import benchpark.repo
import benchpark.runtime
import benchpark.variant

bootstrapper = benchpark.runtime.RuntimeResources(benchpark.paths.benchpark_home)
bootstrapper.bootstrap()

import ramble.language.language_base # noqa
import ramble.language.language_helpers # noqa


class CudaExperiment(Experiment):
"""Auxiliary class which contains CUDA variant, dependencies and conflicts
and is meant to unify and facilitate its usage.
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

This docstring is copied from Experiment and doesn't explain anything new or specific to this class.

variant("cuda", default=False, description="Build and run with CUDA")

#
# These are default values for instance variables.
#

# This allows analysis tools to correctly interpret the class attributes.
variants: Dict[
"benchpark.spec.Spec",
Dict[str, benchpark.variant.Variant],
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should not have to instantiate this here. Why does this inherit from ExperimentSystemBase vs. Experiment? If this is related to MRO concerns, see my later comment about using composition.


def __init__(self, spec):
self.spec: "benchpark.spec.ConcreteExperimentSpec" = spec
super().__init__()

def compute_include_section(self):
# include the config directory
# TODO: does this need to change to interop with System class
return ["./configs"]

def compute_config_section(self):
# default configs for all experiments
return {
"deprecated": True,
"spack_flags": {"install": "--add --keep-stage", "concretize": "-U -f"},
}

# def compute_applications_section(self):
# # TODO: The default in GPU experiments is 1 MPI rank per GPU.
# # What goes here?

def compute_spack_section(self):
# CUDA specific versions
system_specs = {}
system_specs["cuda_version"] = "{default_cuda_version}"
system_specs["cuda_arch"] = "{cuda_arch}"

package_specs = {}
# TODO: help!!
# package_specs["cuda"] = needs_external(

package_specs["cuda"] = {
"pkg_spec": "cuda@{}+allow-unsupported-compilers".format(
system_specs["cuda_version"]
),
"compiler": system_specs["compiler"],
}

package_specs[app_name]["pkg_spec"] += "+cuda cuda_arch={}".format(
system_specs["cuda_arch"]
)

return {
"packages": {k: v for k, v in package_specs.items() if v},
"environments": {app_name: {"packages": list(package_specs.keys())}},
}

def compute_ramble_dict(self):
# This can be overridden by any subclass that needs more flexibility
return {
"ramble": {
"include": self.compute_include_section(),
"config": self.compute_config_section(),
"modifiers": self.compute_modifiers_section(),
"applications": self.compute_applications_section(),
"software": self.compute_spack_section(),
}
}

def write_ramble_dict(self, filepath):
ramble_dict = self.compute_ramble_dict()
with open(filepath, "w") as f:
yaml.dump(ramble_dict, f)
24 changes: 23 additions & 1 deletion lib/benchpark/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,35 @@ def compute_modifiers_section(self):
return [{"name": "allocation"}]

def compute_applications_section(self):
# TODO: is there some reasonable default?
# Require that the experiment defines num_procs
variables = {}
variables["n_ranks"] = self.num_procs

raise NotImplementedError(
"Each experiment must implement compute_applications_section"
)

def needs_external(pkgs_dict, system_specs, pkg_name):
# TODO: how to compose these here?
pkgs_dict[system_specs[pkg_name]] = {}

def compute_spack_section(self):
# TODO: is there some reasonable default based on known variable names?
system_specs = {}
system_specs["compiler"] = "default-compiler"
system_specs["mpi"] = "default-mpi"

package_specs = {}
package_specs[system_specs["mpi"]] = (
{}
) # empty package_specs value implies external package
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like a good candidate for a method in Experiment like

def needs_external(pkgs_dict, system_specs, pkg_name):
  pkgs_dict[system_specs[pkg_name]] = {}

(mainly so this comment doesn't have to be repeated).


# TODO: is there a way to generically do this?
package_specs[app_name] = {
"pkg_spec": f"{app_name}@{app_version} +mpi",
"compiler": system_specs["compiler"],
}

raise NotImplementedError(
"Each experiment must implement compute_spack_section"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this does anything, it can't raise NotImplementedError. FWIW benchpark audit currently makes sure that the lowest-level definition (e.g. what lives in exp_repo) defines this method.

Expand Down
Loading