Oss517

[ankona]

No description provided.

[codecov]

Codecov Report

Merging #356 (011434e) into dashboard (266b3c6) will increase coverage by 0.11%.
The diff coverage is 94.59%.

@@              Coverage Diff              @@
##           dashboard     #356      +/-   ##
=============================================
+ Coverage      90.00%   90.11%   +0.11%     
=============================================
  Files             60       60              
  Lines           3710     3802      +92     
=============================================
+ Hits            3339     3426      +87     
- Misses           371      376       +5     

Files	Coverage Δ
smartsim/_core/config/config.py	98.66% <100.00%> (+0.11%)	⬆️
smartsim/_core/launcher/launcher.py	100.00% <ø> (ø)
smartsim/_core/launcher/local/local.py	96.82% <100.00%> (+2.82%)	⬆️
smartsim/_core/launcher/step/mpiStep.py	88.73% <100.00%> (ø)
smartsim/_core/launcher/step/step.py	100.00% <100.00%> (ø)
smartsim/_core/utils/serialize.py	96.96% <100.00%> (-0.26%)	⬇️
smartsim/entity/dbnode.py	91.96% <100.00%> (-0.15%)	⬇️
smartsim/_core/control/controller.py	86.47% <96.55%> (+0.40%)	⬆️
smartsim/_core/control/job.py	93.93% <95.65%> (+0.51%)	⬆️
smartsim/_core/control/jobmanager.py	93.28% <83.33%> (-0.76%)	⬇️
... and 2 more

[MattToast]

One last thing to consider, and for sake of time I'm going to request this be kicked into a new ticket/PR!

It looks like unmanaged tasks need to launched through the indirect.py entry-point. We are successfully launching everything through the local launcher and that seems to be working perfectly! Unfortunately, it looks like we do not have a similar method to tack unamanged steps that may have been launched via a WLM:

For testing consider the following driver script:

from smartsim import Experiment

def main() -> int:
    slurm = "slurm"      # <-- I believe this problem exists for any WLM
    open_mpi = "mpirun"  #     and run command combo that produces an
                         #     unmanaged step
    exp = Experiment("my-exp", launcher=slurm)

    rs = exp.create_run_settings("echo", ["hello", "world"],
                                 run_command=open_mpi)
    rs.set_tasks(1)
    model = exp.create_model("my-model", run_settings=rs)

    exp.generate(model, overwrite=True)
    exp.start(model, block=True)  # <-- Does not look like any start or
                                  #     stop jsons are produced bc the
                                  #     unmanaged indirect is never invoked
    print("Done")
    return 0

if __name__ == "__main__":
    raise SystemExit(main())

While a slurm/mpirun pairing tends to be wayyyy less popular than the usual slurm/srun configuration, it is not uncommon for users use a PBS/mpi{exec,run} pairing, so it may be worth testing that explicitly as well!

[ankona]

One last thing to consider, and for sake of time I'm going to request this be kicked into a new ticket/PR!

While a slurm/mpirun pairing tends to be wayyyy less popular than the usual slurm/srun configuration, it is not uncommon for users use a PBS/mpi{exec,run} pairing, so it may be worth testing that explicitly as well!

Added a follow-up ticket to give this the love it deserves.

[MattToast]

Looks amazing and seems to be working great! I left a couple suuuupppeer pedantic change requests and some notes for either you/I to write up some tickets for some known defects that we want to address, but nothing worth holding up approval over!

Otherwise LGTM!! Thanks for all of the hard work on this!!

[MattToast]

Same "don't love, but good enough for now" concern with param diving the telemetry_dir: @MattToast and/or @ankona remember to ticket this as well!!

[ankona]

+1