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.

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

Jump to bottom

Update pegasus_workflow.py #4936

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Update pegasus_workflow.py #4936

merged 2 commits into from
Nov 14, 2024

Conversation

sebastiangomezlopez
Copy link
Contributor

@sebastiangomezlopez sebastiangomezlopez commented Nov 13, 2024 •
edited
Loading

Bugfix in pegasus_workflow.py

This fix implements a workaround for cases where PyCBC and/or PyGRB workflows have a subworkflow structure that forbid the
pegasus-version command from being successfully executed on working nodes.

This is a: bug fix.

This change affects: the offline search, PyGRB.

@ahnitz
Copy link
Member

ahnitz commented Nov 13, 2024

Is this because pegasus (the full version) is not actually installed on worker nodes? I just want to make sure I understand where this comes up.

ahnitz
ahnitz reviewed Nov 13, 2024
View reviewed changes
pycbc/workflow/pegasus_workflow.py
if version.parse(sproc_out) >= version.parse('5.0.4'):
try:
sproc_out = subprocess.check_output(['pegasus-version']).strip()
sproc_out = sproc_out.decode()
Copy link
Member

Choose a reason for hiding this comment

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

@spxiwh Is there a reason we don't just require >= 5.0.4 anyway?

Copy link
Contributor

Choose a reason for hiding this comment

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

No real reason why we couldn't do that.

@spxiwh
Copy link
Contributor

spxiwh commented Nov 14, 2024

Is this because pegasus (the full version) is not actually installed on worker nodes? I just want to make sure I understand where this comes up.

Yes. The worker package pegasus ships to nodes does not contain a bunch of stuff, and pegasus-version does not work. PyGRB is trying to run a workflow generator during another running workflow that itself contains subworkflows ... and that particular combination is something we never tested and was broken by the line being changed here.

spxiwh
spxiwh approved these changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@spxiwh spxiwh left a comment

Choose a reason for hiding this comment

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

Good from my perspective. Thanks!

@sebastiangomezlopez sebastiangomezlopez merged commit 3efc80c into gwastro:master Nov 14, 2024
29 checks passed
prayush pushed a commit to prayush/pycbc that referenced this pull request Nov 21, 2024
@sebastiangomezlopez @titodalcanton @prayush
Update pegasus_workflow.py (gwastro#4936)
011420b 
* Update pegasus_workflow.py

* Update pycbc/workflow/pegasus_workflow.py

Co-authored-by: Tito Dal Canton <[email protected]>

---------

Co-authored-by: Tito Dal Canton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@titodalcanton titodalcanton titodalcanton left review comments

@ahnitz ahnitz ahnitz left review comments

@spxiwh spxiwh spxiwh approved these changes

Assignees

@pannarale pannarale

@sebastiangomezlopez sebastiangomezlopez

Labels
bug offline search PyGRB PyGRB development
Projects
PyGRB Development
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sebastiangomezlopez @ahnitz @spxiwh @titodalcanton @pannarale