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

BaseJobGroup, JobGroup, ColocatedJobGroup skeleton #599

Merged

Conversation

amandarichardsonn
Copy link
Contributor

No description provided.

@amandarichardsonn amandarichardsonn changed the title JobGroup and JobGroup Implementation May 22, 2024
@amandarichardsonn amandarichardsonn changed the title JobGroup Implementation BaseJobGroup, JobGroup, ColocatedJobGroup Implementation May 23, 2024
@amandarichardsonn amandarichardsonn changed the title BaseJobGroup, JobGroup, ColocatedJobGroup Implementation BaseJobGroup, JobGroup, ColocatedJobGroup skeleton May 23, 2024
@amandarichardsonn amandarichardsonn added ignore-for-release type: feature Issues that include feature request or feature idea labels May 23, 2024
@amandarichardsonn amandarichardsonn self-assigned this May 23, 2024
Copy link
Contributor

@juliaputko juliaputko left a comment

Choose a reason for hiding this comment

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

lgtm

job_group = ColocatedJobGroup([job_1])
assert len(job_group) == 1

# compare job names when Job class is up, handleful of other atts, not reused elsewhere
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you test these now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes nice catch!

job_group = JobGroup([job_1])
assert len(job_group) == 1

# cannot test setitem until Job is implemented since there is no comparison bc of the deep copy
Copy link
Contributor

Choose a reason for hiding this comment

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

same with these?

Copy link
Contributor

@juliaputko juliaputko left a comment

Choose a reason for hiding this comment

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

I think that all looks good!

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 63.79310% with 21 lines in your changes missing coverage. Please review.

Please upload report for BASE (smartsim-refactor@b64af80). Learn more about missing BASE report.

Files with missing lines Patch % Lines
smartsim/launchable/baseJobGroup.py 62.50% 9 Missing ⚠️
smartsim/launchable/colocatedJobGroup.py 64.28% 5 Missing ⚠️
smartsim/launchable/jobGroup.py 64.28% 5 Missing ⚠️
smartsim/launchable/mpmdpair.py 33.33% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                  @@
##             smartsim-refactor     #599   +/-   ##
====================================================
  Coverage                     ?   31.76%           
====================================================
  Files                        ?       87           
  Lines                        ?     6083           
  Branches                     ?        0           
====================================================
  Hits                         ?     1932           
  Misses                       ?     4151           
  Partials                     ?        0           
Files with missing lines Coverage Δ
smartsim/launchable/__init__.py 100.00% <100.00%> (ø)
smartsim/launchable/basejob.py 80.00% <ø> (ø)
smartsim/launchable/mpmdpair.py 71.42% <33.33%> (ø)
smartsim/launchable/colocatedJobGroup.py 64.28% <64.28%> (ø)
smartsim/launchable/jobGroup.py 64.28% <64.28%> (ø)
smartsim/launchable/baseJobGroup.py 62.50% <62.50%> (ø)

@amandarichardsonn amandarichardsonn merged commit d058213 into CrayLabs:smartsim-refactor Jun 11, 2024
34 of 35 checks passed
@amandarichardsonn amandarichardsonn deleted the job-group branch June 11, 2024 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release type: feature Issues that include feature request or feature idea
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants