Skip to content

misc fixes #280

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.

Sign up for GitHub

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

Merged
akoumpa merged 5 commits into from
Jul 2, 2025
Merged

misc fixes #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3f865a3
guard None env
akoumpa Jul 1, 2025
dda66eb
add enable_goodbye_message flag
akoumpa Jul 1, 2025
97afe02
write sbatch script if it does not exist
akoumpa Jul 1, 2025
cc1fccd
Update slurm.py
akoumpa Jul 1, 2025
22527d0
Update slurm.py
akoumpa Jul 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions nemo_run/run/experiment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@
jobs: list[Job | JobGroup] | None = None,
base_dir: str | None = None,
clean_mode: bool = False,
enable_goodbye_message: bool = True,
) -> None:
"""
Initializes an experiment run by creating its metadata directory and saving the experiment config.
Expand All @@ -317,6 +318,7 @@
_reconstruct: Generally, the user does not need to specify this flag.
This is only set to True when using run.Experiment.from_dir.
clean_mode: If True, disables all console output (logs, progress bars, etc.). Defaults to False.
enable_goodbye_message: if True, prints goodbye message after submitting job. Defaults to True.
"""
configure_logging(level=log_level)
self._reconstruct = _reconstruct
Expand All @@ -325,6 +327,7 @@

self._title = title
self._id = id or f"{title}_{int(time.time())}"
self._enable_goodbye_message = enable_goodbye_message

base_dir = str(base_dir or get_nemorun_home())
self._exp_dir = os.path.join(base_dir, "experiments", title, self._id)
Expand Down Expand Up @@ -1181,16 +1184,17 @@
theme=os.environ.get("NEMO_RUN_CODE_THEME", "monokai"),
)
)
self.console.print(
Syntax(
self.GOODBYE_MESSAGE_BASH.format(
exp_id=self._id,
tasks=list(map(lambda job: job.id, self.jobs)),
),
"shell",
theme=os.environ.get("NEMO_RUN_CODE_THEME", "monokai"),
if self._enable_goodbye_message:
self.console.print(
Syntax(
self.GOODBYE_MESSAGE_BASH.format(
exp_id=self._id,
tasks=list(map(lambda job: job.id, self.jobs)),

Check warning

Code scanning / CodeQL

'break' or 'return' statement in finally Warning

'return' in a finally block will swallow any exceptions raised.

Copilot Autofix

AI 9 months ago

To fix the issue, the return statement should be moved outside the finally block. This ensures that exceptions raised during the execution of the try block or the finally block are not suppressed. The cleanup operations in the finally block will still execute, but the exception will propagate correctly.

The best approach is to refactor the __exit__ method to store the return value in a variable and return it after the finally block. This preserves the intended functionality while ensuring proper exception handling.

Suggested changeset 1
nemo_run/run/experiment.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/nemo_run/run/experiment.py b/nemo_run/run/experiment.py
--- a/nemo_run/run/experiment.py
+++ b/nemo_run/run/experiment.py
@@ -1136,2 +1136,3 @@
     def __exit__(self, exc_type, exc_value, tb):
+        return_value = None
         try:
@@ -1146,5 +1147,5 @@
                     self.status()
-                return
+                return_value = None
 
-            if self._launched:
+            elif self._launched:
                 if hasattr(self, "_direct") and self._direct:
@@ -1154,5 +1155,5 @@
                     self.status()
-                    return
+                    return_value = None
 
-                if hasattr(self, "_waited") and self._waited:
+                elif hasattr(self, "_waited") and self._waited:
                     self.console.rule(
@@ -1161,10 +1162,10 @@
                     self.status()
-                    return
-
-                self.console.rule(
-                    f"[bold magenta]Waiting for Experiment {self._id} to finish",
-                )
-                self.status()
+                    return_value = None
 
-                self._wait_for_jobs(jobs=self.jobs)
+                else:
+                    self.console.rule(
+                        f"[bold magenta]Waiting for Experiment {self._id} to finish",
+                    )
+                    self.status()
+                    self._wait_for_jobs(jobs=self.jobs)
         finally:
@@ -1197,2 +1198,3 @@
                     )
+        return return_value
 
EOF
@@ -1136,2 +1136,3 @@
def __exit__(self, exc_type, exc_value, tb):
return_value = None
try:
@@ -1146,5 +1147,5 @@
self.status()
return
return_value = None

if self._launched:
elif self._launched:
if hasattr(self, "_direct") and self._direct:
@@ -1154,5 +1155,5 @@
self.status()
return
return_value = None

if hasattr(self, "_waited") and self._waited:
elif hasattr(self, "_waited") and self._waited:
self.console.rule(
@@ -1161,10 +1162,10 @@
self.status()
return

self.console.rule(
f"[bold magenta]Waiting for Experiment {self._id} to finish",
)
self.status()
return_value = None

self._wait_for_jobs(jobs=self.jobs)
else:
self.console.rule(
f"[bold magenta]Waiting for Experiment {self._id} to finish",
)
self.status()
self._wait_for_jobs(jobs=self.jobs)
finally:
@@ -1197,2 +1198,3 @@
)
return return_value

Copilot is powered by AI and may make mistakes. Always verify output.
),
"shell",
theme=os.environ.get("NEMO_RUN_CODE_THEME", "monokai"),
)
)
)

def _repr_svg_(self):
return self.to_config()._repr_svg_()
Expand Down
3 changes: 2 additions & 1 deletion nemo_run/run/torchx_backend/packaging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ def _get_details_from_script(fn_or_script: Script, serialize_configs: bool):
fn_or_script, serialize_configs=True
)
metadata = fn_or_script.metadata
env = env | fn_or_script.env
if fn_or_script.env:
env = env | fn_or_script.env

launcher = executor.get_launcher()
if executor.supports_launcher_transform():
Expand Down
Loading