-
Notifications
You must be signed in to change notification settings - Fork 137
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.
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
ensemble model parallelism #1342
Conversation
Job Mingw Test on 32960e5 : invalidated by @joshua-cogliati-inl test civet |
1 similar comment
Job Mingw Test on 32960e5 : invalidated by @joshua-cogliati-inl test civet |
@joshua-cogliati-inl ready to review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some things that need to be fixed. I would prefer if this was split into two pull requests, but that is not necessary. I also had some questions.
There are definitely improvements in this pull request :)
tests/framework/CodeInterfaceTests/Instant/IAEA2D/sample/1/alias.xml
Outdated
Show resolved
Hide resolved
@joshua-cogliati-inl Josh I addressed your comments |
Job Test Ubuntu 18-2 Python 3 on da1cd4f : invalidated by @joshua-cogliati-inl failed tests/framework/Optimizers/SimulatedAnnealing/ExponentialEggHolder |
Any hope for a end-of-week miracle merge on this? |
@PaulTalbot-INL looks like Josh is on it...so you called the miracle ehhehe |
I approve, but this changes a requirements test so needs a second approval from the CCB chair or designee. |
@PaulTalbot-INL @alfoa This changes a requirements test, so it needs a second approver. |
As chair of COB I approve the requirement test modifications. |
@PaulTalbot-INL can you also approve the PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with all these changes, as well.
kwargsKeys.pop(kwargsKeys.index("jobHandler")) | ||
kwargsToKeep = { keepKey: kwargs[keepKey] for keepKey in kwargsKeys} | ||
jobHandler = kwargs['jobHandler'] | ||
kwargsToKeep = { keepKey: kwargs[keepKey] for keepKey in list(kwargs.keys())} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just kwargsToKeep == copy.deepcopy(kwargs)
? No change needed, I see how this could be useful in the future if we have keys we don't want to keep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ops...yes.. i will address this in next PR
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
Closes #1341
What are the significant changes in functionality due to this change request?
Changed the parallelism strategy in case of ensemble model.
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.
<internalParallel>
to True. (no significant functionality added)raven/tests/framework/user_guide
andraven/docs/workshop
) have been changed, the associated documentation must be reviewed and assured the text matches the example. (no change)