Skip to content

Dvm concurrency amelioration - #264

Merged
markcoletti merged 8 commits into
0.9.0-prereleasefrom
dvm-concurrency-amelioration
Oct 29, 2026
Merged

markcoletti merged 8 commits into
0.9.0-prereleasefrom
dvm-concurrency-amelioration

Conversation

@markcoletti

Copy link
Copy Markdown

This incorporates @Lance-Drane 's fixes that were on the 0.9.0-prerelease branch and adds copious DVM related logging. I also made some minor changes to the examples yet again to convey that they examples when viewed on the portal.

Comment thread ipsframework/services.py
# writes the URI to a file, and we need to read it to get the
# URI. We wait for the file to be created and for the URI to be
# written to it. We wait up to 30 seconds for this
deadline = time.monotonic() + 30

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

good catch on time.monotonic(), I might have to refactor some of my code to use this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This was yet another instance of "I didn't know to know" courtesy of the LLM. To me, that's one of the best unexpected benefits of using those tools is learning of concepts or tools that I needed to know, or are otherwise useful, but didn't.

@Lance-Drane Lance-Drane left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, might need to run this through the formatter/linter and add a couple of typing checks but that's no reason to block

Comment thread ipsframework/services.py
class _DVMDiagnostics:
"""Worker-local, thread-safe lifecycle journal; constructed after plugin transfer."""

def __init__(self, directory, worker):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

might be nice to add types here (Path and Worker should already be imported)

Comment thread ipsframework/services.py
Comment on lines +3398 to +3406
environment={
key: os.environ[key]
for key in (
'SLURM_JOB_ID', 'SLURM_JOB_NODELIST', 'SLURM_JOB_NUM_NODES',
'PRTE_MCA_ras_slurm_use_entire_allocation',
'PRTE_MCA_rmaps_default_mapping_policy',
)
if key in os.environ
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

optional consideration: do you think it might be useful to just greedily record every environment variable which starts with SLURM_ or PRTE_ ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It can't hurt?

@markcoletti
markcoletti merged commit 0121888 into 0.9.0-prerelease Oct 29, 2026
13 of 15 checks passed
@markcoletti
markcoletti deleted the dvm-concurrency-amelioration branch October 29, 2026 14:46
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.

2 participants