Conversation
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #84 +/- ##
==========================================
- Coverage 82.88% 82.39% -0.49%
==========================================
Files 11 12 +1
Lines 777 835 +58
==========================================
+ Hits 644 688 +44
- Misses 133 147 +14
🚀 New features to boost your workflow:
|
| base += [""] | ||
| return "\n".join(base) | ||
|
|
||
| def _export_parallel_vars(self) -> str: |
There was a problem hiding this comment.
These are only being exported so that they can be consumed when generating the shared args right? Can we just populate the parameters directly instead?
There was a problem hiding this comment.
True, will remove this.
There was a problem hiding this comment.
forgot to tag, fixed this.
pyproject.toml
Outdated
| authors = [{name = "Marshall Wang", email = "marshall.wang@vectorinstitute.ai"}] | ||
| license = "MIT" | ||
| requires-python = ">=3.10" | ||
| requires-python = ">=3.10,<4.0" |
There was a problem hiding this comment.
Yes, but when I didn't have it, it was trying to use Python 3.12 and ran into dependency issues. That's why I changed it. But I guess we can leave that for another PR if it's still a problem.
I was still using Poetry.
There was a problem hiding this comment.
This file is outdated and is of no use at the moment, might retire it in the future, I'd say just leave it as is for now, especially now we're no longer using poetry
|
|
||
| def _export_parallel_vars(self) -> str: | ||
| if self.is_multinode: | ||
| return """if [ "$PIPELINE_PARALLELISM" = "True" ]; then |
There was a problem hiding this comment.
Since we're generating these scripts dynamically, we shouldn't need to have any if else statements in the generated scripts. This logic should go directly into _generate_shared_args, which it looks like you have the same checks in that function already.
There was a problem hiding this comment.
forgot to tag, fixed this.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…v var usage in generated slurm script, change non env var name to lower case
for more information, see https://pre-commit.ci
4 participants
This pull request includes dependency updates and the addition of a new
SlurmScriptGeneratorclass for generating SLURM scripts.requires-pythonfield inpyproject.toml.set_env_varsmethod invec_inf/cli/_helper.py.SlurmScriptGeneratorclass invec_inf/cli/_slurm_script_generator.pyto dynamically generate SLURM scripts based on the parameters provided.