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

Generator class refactor and inject path into Dragon & Shell Launcher #634

Conversation

amandarichardsonn
Copy link
Contributor

@amandarichardsonn amandarichardsonn commented Jul 16, 2024

New PR located at: https://github.com/MattToast/SmartSim/pull/3#issue-2414877973

@amandarichardsonn amandarichardsonn added area: job Issues related to job management area: generation Issues related to Experiment file generation ignore-for-release labels Jul 16, 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.

Couldn't find anything glaring. You've got quite a few TODO's that youre probably aware of, and the additional stuff that you'll add once the file operations are merged in. Super cool stuff!

msg=f"Configured application {entity.name} with no parameters",
)

# if entity.files:
Copy link
Contributor

Choose a reason for hiding this comment

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

remove comments?

@@ -71,7 +74,7 @@ def unpack_colo_fs_identifier(fs_id: str) -> str:
return "_" + fs_id if fs_id else ""


def create_short_id_str() -> str:
def create_short_id_str() -> str: # here
Copy link
Contributor

Choose a reason for hiding this comment

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

remove comment

exe, *rest = launchable
self._launched[id_] = sp.Popen(
(helpers.expand_exe_path(exe), *rest), cwd=job_execution_path
) # can specify a different dir for Popen
Copy link
Contributor

Choose a reason for hiding this comment

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

change to a todo or remove comment?

"""Test if the log level is DEBUG when environment variable is set to "debug"."""
environ["SMARTSIM_LOG_LEVEL"] = "debug"
assert gen_instance_for_job.log_level == DEBUG
# Clean up: unset the environment variable
Copy link
Contributor

Choose a reason for hiding this comment

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

address these?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: generation Issues related to Experiment file generation area: job Issues related to job management ignore-for-release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants