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

Treat DragonSteps as a Managed Steps #6

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

MattToast
Copy link
Collaborator

The DragonLauncher now maps dragon run settings to a DragonStep and regular run settings to a LoacalStep to match behavior of existing launchers.

The DragonLauncher treats DragonSteps as managed steps and LocalSteps as unmanaged steps. The DragonSteps run commands are now tracked natively through the dragon run time and not proxyed through the proxyable_run_cmd decorator.

Remove unnecessary down casts to dragon specific entities. Unify connect_to_dragon code path for the main SmartSim process and the telemetry monitor.

The `DragonLauncher` now maps dragon run settings to a `DragonStep` and
regular run settings to a `LoacalStep` to match behavior of existing
launchers. The `DragonLauncher` treats `DragonStep`s as managed steps
and `LocalStep`s as unmanaged steps. The `DragonStep`s run commands are
now tracked natively through the dragon run time and not proxyed
through the `proxyable_run_cmd` decorator. Remove unnecessary down casts
to dragon specific entities. Unify `connect_to_dragon` code path for
main SmartSim process and the telemetry monitor.
Hack "fix" for race condition in telemetry monitor test. This ought to
be removed before we officially ship a dragon launcher!
@MattToast MattToast self-assigned this Mar 8, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2024

Codecov Report

Attention: Patch coverage is 16.27907% with 36 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (protodrg@4f9f72e). Click here to learn what that means.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             protodrg       #6   +/-   ##
===========================================
  Coverage            ?   83.79%           
===========================================
  Files               ?       70           
  Lines               ?     4395           
  Branches            ?        0           
===========================================
  Hits                ?     3683           
  Misses              ?      712           
  Partials            ?        0           
Files Coverage Δ
smartsim/_core/launcher/step/step.py 98.46% <ø> (ø)
smartsim/_core/control/controller.py 86.41% <0.00%> (ø)
smartsim/_core/launcher/step/dragonStep.py 39.39% <37.50%> (ø)
smartsim/_core/launcher/dragon/dragonLauncher.py 29.81% <11.76%> (ø)

Co-authored-by: Matt Drozt <matthew.drozt@gmail.com>
Copy link
Owner

@al-rigazzi al-rigazzi left a comment

Choose a reason for hiding this comment

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

LGTM, one step closer to deliverable!

@al-rigazzi al-rigazzi merged commit 372e152 into al-rigazzi:protodrg Mar 14, 2024
32 checks passed
@MattToast MattToast deleted the drg-managed-steps branch March 29, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants