features/branson

[rfhaque]

Description

Adding a specification of Branson https://lanl.github.io/benchmarks/01_branson/branson.html.

We should work with @gshipman and @alexrlongne to make progress on incorporating Branson.

• Type of PR: Adding an experiment
• Add a new application.py and (maybe) package.py under a new directory for this benchmark
• Add an experiment.py
• Define an exec_mode=test and exec_mode=perf experiment
• Change the git location in package.py once the source PR Update cmake script lanl/branson#53 is merged
• Add to gitlab on 3 systems

[rfhaque]

@pearce8 This is just a temporary fix for lassen. Need to work with the branson team to fix build issues on that system.

[alexrlongne]

I'll look at the RNG package again and see if an update fixes this.

[pearce8]

Please add a dry run.

[scheibelp]

minor compatibility issue w/ #953 (should be resolved w/ merge from develop)

[slabasan]

@scheibelp NotImplementedError: "branson" cannot run with MPI only without inheriting from MpiOnlyExperiment. Choose from ['rocm', 'cuda', 'openmp']

[scheibelp]

I think this was just missing the MpiOnlyExperiment logic added in ce3ffb6#diff-9607e034d65374d4f8618d80af3e2d5033d973bdf09932cb07b5981232425232 - all the dry runs pass now.

I think someone other than me has to approve because I submitted the latest commits to this PR @slabasan, but I am adding an approving review to say the other parts look good to me.

[rfhaque]

@scheibelp Does sparta need to provide an mpi variant for this to work? mpi is always on in sparta and thus a variant is not needed

[scheibelp]

It appears to be necessary based on the logic added by ce3ffb6 but @michaelmckinsey1 am I missing a way for a package that always uses mpi to avoid this?

[michaelmckinsey1]

It appears to be necessary based on the logic added by ce3ffb6 but @michaelmckinsey1 am I missing a way for a package that always uses mpi to avoid this?

@scheibelp All programming models run with MPI, but to run only with MPI the experiment must inherit from MpiOnlyExperiment. This is to differentiate between experiments that cannot be ran only with MPI experiment+mpi, such as babelstream+mpi. babelstream+mpi+cuda, babelstream+mpi+rocm, babelstream+mpi+openmp are all valid, however babelstream+mpi is not. I believe babelstream may be the only current experiment with this behavior.

[scheibelp]

I think what @rfhaque is saying is that it would never make sense to experiment init branson ~mpi, so the presence of an mpi variant is confusing.

@michaelmckinsey1 I think that means that if an experiment init branson would fail based on the logic in https://github.com/LLNL/benchpark/blob/develop/lib/benchpark/experiment.py#L279 (it wants mpi to be mentioned explicitly).

I was thinking perhaps https://github.com/LLNL/benchpark/blob/develop/.github/utils/dryruns.py was dispatching the wrong call, but it seems like it was generating the appropriate experiment init branson for e830b9d, but that it was rejected by the logic in experiment.py.

Is it your opinion such experiments should have an mpi variant? Or am I missing a way for an experiment to be set up this way?

[michaelmckinsey1]

The mpi variant is true by default, so that is why experiment init branson specs to branson+mpi without needing to explicitly write +mpi. I think it makes sense to have an explicit mpi variant because of the babelstream example. I don't see the test where the logic is failing, but I can take a look if it is.

[slabasan]

LGTM, but waiting on the package.py to be updated upstream?

[michaelmckinsey1]

This PR raised some confusion about the MpiOnlyExperiment mechanism. It is described #904 and in the docs for writing an experiment. It should be generally included for every experiment, except cases like babelstream, which can not be ran for babelstream+mpi, but can be ran for babelstream+mpi+cuda. So it is used to indicate whether running +mpi without any programming models is valid.

Inheriting from MpiOnlyExperiment is used to check spec validity

benchpark/lib/benchpark/experiment.py

Lines 259 to 296 in dd1b469

	# Explicitly ordered list. "mpi" first
	models = ["mpi"] + ["openmp", "cuda", "rocm"]
	invalid_models = []
	for model in models:
	# Experiment specifying model in add_package_spec that it doesn't implement
	if (
	self.spec.satisfies("+" + model)
	and model not in self.programming_models
	):
	invalid_models.append(model)
	# Case where there are no experiments specified in experiment.py
	if len(self.programming_models) == 0:
	raise NotImplementedError(
	f"Please specify a programming model in your {self.name}/experiment.py (e.g. MpiOnlyExperiment, OpenMPExperiment, CudaExperiment, ROCmExperiment). See other experiments for examples."
	)
	elif len(invalid_models) > 0:
	raise NotImplementedError(
	f'{invalid_models} are not valid programming models for "{self.name}". Choose from {self.programming_models}.'
	)
	# Check if experiment is trying to run in MpiOnly mode without being an MpiOnlyExperiment
	elif "mpi" not in str(self.spec) and not any(
	self.spec.satisfies("+" + model) for model in models[1:]
	):
	raise NotImplementedError(
	f'"{self.name}" cannot run with MPI only without inheriting from MpiOnlyExperiment. Choose from {self.programming_models}'
	)
	if (
	sum([self.spec.satisfies(s) for s in ["+strong", "+weak", "+throughput"]])
	> 1
	):
	raise BenchparkError(
	f"spec cannot specify multiple scaling options. {self.spec}"
	)
	if sum([self.spec.satisfies(s) for s in ["+cuda", "+rocm", "+openmp"]]) > 1:
	raise BenchparkError(
	f"spec cannot specify multiple mutually-exclusive programming models. {self.spec}"
	)

. #931 may drive some changes to MpiOnlyExperiment.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.43%. Comparing base (8672555) to head (0fd8824).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #462      +/-   ##
===========================================
+ Coverage    65.30%   65.43%   +0.12%     
===========================================
  Files           44       44              
  Lines         3240     3240              
  Branches       256      256              
===========================================
+ Hits          2116     2120       +4     
+ Misses        1117     1113       -4     
  Partials         7        7              

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.