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

Added section on overriding lp config in loop #5223

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Added section on overriding lp config in loop #5223

merged 5 commits into from
Apr 15, 2024

Conversation

pryce-turner
Copy link
Contributor

Small enhancements to nested parallelization docs showing how one can override default workflow params

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. documentation Improvements or additions to documentation labels Apr 11, 2024
neverett
neverett previously approved these changes Apr 11, 2024
View reviewed changes
Copy link
Contributor

@neverett neverett left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One small suggestion, otherwise LGTM! We're not very consistent with how we refer to launch plans in the docs, but have been trying to move in the direction of two separate words.

docs/user_guide/advanced_composition/nested_parallelization.md Outdated Show resolved Hide resolved
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 11, 2024
@pryce-turner pryce-turner enabled auto-merge (squash) April 12, 2024 18:10
@pryce-turner
Copy link
Contributor Author

@neverett would you mind reviewing again please since adding that suggestion voided the first one..

neverett
neverett previously approved these changes Apr 12, 2024
View reviewed changes
@pryce-turner
Copy link
Contributor Author

@neverett is this the same failure we saw last time? Do you know how to get past it?

@neverett
Copy link
Contributor

@pryce-turner I think if you merge master into your branch, that should fix the error, since it was fixed in this PR: #5215 -- let me know if it's not fixed after that, though!

@pryce-turner
Copy link
Contributor Author

@neverett looks like it dismissed your review again.. I actually fixed the code a bit so that makes sense. The failing check should pass now though if you could please checkmark this one more time.

@pryce-turner pryce-turner merged commit d0ed6c4 into master Apr 15, 2024
46 checks passed
@pryce-turner pryce-turner deleted the pryce-turner/add-lp-parallelization branch April 15, 2024 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neverett neverett neverett approved these changes

@ppiegaze ppiegaze Awaiting requested review from ppiegaze ppiegaze is a code owner

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pryce-turner @neverett