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

Add raja perf benchmarks #2

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Add raja perf benchmarks #2

wants to merge 16 commits into from

Conversation

johnbowen42
Copy link
Contributor

No description provided.

driver.py Outdated
@@ -211,12 +220,10 @@ def build_and_run(self, reps, profiler=None):
or self.exemode == "jitify"
), "Expected aot or proteus or jitify for exemode"

exe = f"{self.benchmark}-{self.exemode}.x"
self.clean()
#self.clean()
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC for hecbench we have to clean first because otherwise different compilation modes (aot, proteus) pick up pre-built, incorrect object files.

I think we should reinstate cleaning and add a clean key in the per benchmark or benchmark grouping if possible.

# the build command is meant to be a full bash command to build the benchmark, eg
# `cmake -DCMAKE_BUILD_TYPE=Debug --build` or `make benchmark`
# If none is provided, it will default to `make`
self.build_command = 'make' if build_command == None else build_command
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the default. User should define explicitly. We should use the TOML dictionary hierarchically, so user can provide a build command that applies to a group of programs.

driver.py Outdated
config["inputs"],
args.compiler,
args.proteus_path,
env_configs,
env_configs
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Black formatter or Ruff. It requires the comma here.

Comment on lines +473 to +474
for env in env_configs:
env["PROTEUS_INSTALL_PATH"] = proteus_install
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is that used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

build instructions for RAJA perf

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume programs are commented out because you are testing?

#[ATOMIC.nvidia.aot]
#path = "benchmarks/RAJAPerf"
#exe = "./build/bin/raja-perf.exe"
#args = "--exclude-variants Base_Seq Lambda_Seq RAJA_Seq Base_CUDA RAJA_CUDA "
Copy link
Contributor

Choose a reason for hiding this comment

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

Two questions:

  1. Why do we do --exclude-variants instead of doing --variant to only run the desired one?
  2. Why do you exclude RAJA_CUDA? That's the one we want to run without proteus enabled (same for HIP)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Legacy, I can refactor but it's a little tedious
  2. I am comparing RAJA_CUDA (which always uses proteus) to Lambda_CUDA in these benchmarks, as a hack to avoid modifying the benchmarks heavily to have an extra RAJA_CUDA_JIT variant. Lambda_CUDA and RAJA_CUDA are roughly on parity (by design) for these benchmarks. This can be fixed later and we can add a RAJA_CUDA_JIT variant to RAJA perf

@@ -0,0 +1,1065 @@
[build]
[build.nvidia]
Copy link
Contributor

Choose a reason for hiding this comment

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

We need build configurations with/without proteus

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see the above comment, I don't think we do. Ultimately we will add forall_jit to RAJA, and a RAJA_CUDA_JIT/RAJA_HIP_JIT variant to RAJAPerf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants