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

[pearce8]

Please add a dry run.

[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 https://github.com/LLNL/benchpark/blob/dd1b469c84f23ad1bd334762a145cb0cb7b416a0/lib/benchpark/experiment.py#L259-L296. #931 may drive some changes to MpiOnlyExperiment.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.37%. Comparing base (822dcfa) to head (821d0f4).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #462      +/-   ##
===========================================
+ Coverage    63.25%   63.37%   +0.11%     
===========================================
  Files           45       45              
  Lines         3410     3410              
  Branches       261      261              
===========================================
+ Hits          2157     2161       +4     
+ Misses        1244     1240       -4     
  Partials         9        9              

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.