Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove Cobalt support #448

Merged
merged 8 commits into from
Jan 19, 2024
Merged

Remove Cobalt support #448

merged 8 commits into from
Jan 19, 2024

Conversation

al-rigazzi
Copy link
Collaborator

With the retirement of Theta, we are not aware of any other system using Cobalt as a workload manager. Therefore, this PR removes support for Cobalt.

Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f683521) 90.34% compared to head (51715d7) 90.80%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #448      +/-   ##
===========================================
+ Coverage    90.34%   90.80%   +0.46%     
===========================================
  Files           60       60              
  Lines         3833     3830       -3     
===========================================
+ Hits          3463     3478      +15     
+ Misses         370      352      -18     
Files Coverage Δ
smartsim/_core/control/controller.py 87.20% <100.00%> (+0.32%) ⬆️
smartsim/_core/control/manifest.py 96.25% <100.00%> (+2.50%) ⬆️
smartsim/_core/launcher/__init__.py 100.00% <ø> (ø)
smartsim/_core/launcher/step/__init__.py 100.00% <ø> (ø)
smartsim/_core/launcher/step/mpiStep.py 88.88% <100.00%> (ø)
smartsim/_core/launcher/stepInfo.py 100.00% <ø> (ø)
smartsim/database/orchestrator.py 86.85% <ø> (ø)
smartsim/entity/dbobject.py 65.13% <100.00%> (ø)
smartsim/experiment.py 85.62% <100.00%> (+0.28%) ⬆️
smartsim/log.py 93.63% <100.00%> (ø)
... and 3 more

... and 5 files with indirect coverage changes

Copy link
Member

@MattToast MattToast left a comment

Choose a reason for hiding this comment

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

Everything seems to LGTM!!

Copy link
Collaborator

@ashao ashao left a comment

Choose a reason for hiding this comment

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

Looks good! The only suggestion, which may have been discussed while I was away, is to throw a more informative error if someone specifically tried to request the cobalt launcher. I wouldn't even give it it's own dummy class, just throw it on the instantiation of Experiment, i.e.

if launcher == "cobalt":
  raise SSUnsupportedError("Support for the Cobalt launcher is no longer supported")

@al-rigazzi
Copy link
Collaborator Author

Looks good! The only suggestion, which may have been discussed while I was away, is to throw a more informative error if someone specifically tried to request the cobalt launcher. I wouldn't even give it it's own dummy class, just throw it on the instantiation of Experiment, i.e.

if launcher == "cobalt":
  raise SSUnsupportedError("Support for the Cobalt launcher is no longer supported")

Slightly reworded and added to Experiment. Added a test.

@al-rigazzi al-rigazzi merged commit e107932 into CrayLabs:develop Jan 19, 2024
26 checks passed
@al-rigazzi al-rigazzi deleted the drop_cobalt branch January 19, 2024 18:35
ashao pushed a commit to ashao/SmartSim that referenced this pull request Jan 27, 2024
As we are not aware of any system still using the Cobalt workload manager, its support in SmartSim was terminated.

[ committed by @al-rigazzi ]
[ reviewed by @MattToast @ashao ]
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants