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

#2130: Add spec for skipped and identical phases #2137

Merged
merged 4 commits into from
May 16, 2023
Merged

#2130: Add spec for skipped and identical phases #2137

merged 4 commits into from
May 16, 2023

Conversation

thearusable
Copy link
Contributor

Closes #2130

@thearusable thearusable self-assigned this Apr 25, 2023
@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch from 488faa8 to 4dcc651 Compare April 25, 2023 16:25
@thearusable thearusable marked this pull request as draft April 25, 2023 16:34
@github-actions
Copy link

github-actions bot commented Apr 25, 2023 •
edited
Loading

Pipelines results

PR tests (clang-9, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-9, ubuntu, mpich, zoltan, json schema test)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-12, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-11, ubuntu, mpich, trace runtime, coverage)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for 4dcc651 (2023-04-25 16:25:21 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.2, ubuntu, mpich)

Build for b9fe835 (2023-05-10 08:56:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.2, gcc-9, ubuntu, mpich)

Build for 526a6ed (2023-05-16 12:34:09 UTC)

Compilation - successful

Testing - passed

Build log

@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch from 4dcc651 to becd546 Compare April 26, 2023 12:34
@thearusable thearusable marked this pull request as ready for review April 26, 2023 12:34
lifflander
lifflander approved these changes May 3, 2023
View reviewed changes
Copy link
Collaborator

@lifflander lifflander left a comment

Choose a reason for hiding this comment

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

I think this looks good. The next step is to use this data to determine for LBDataRestartReader when a phase is identical or skipped and use to to orchestrate the object distribution.

@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch from becd546 to 6d43913 Compare May 4, 2023 12:19
@thearusable thearusable mentioned this pull request May 9, 2023
Merged
@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch from bc4d0b9 to 614a1d2 Compare May 9, 2023 16:21
nlslatt
nlslatt requested changes May 9, 2023
View reviewed changes
Copy link
Collaborator

@nlslatt nlslatt left a comment

Choose a reason for hiding this comment

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

There are test failures that need to be resolved.

tests/unit/collection/test_lb_data_holder.cc Outdated Show resolved Hide resolved
tests/unit/collection/test_lb_data_holder.cc Outdated Show resolved Hide resolved
tests/unit/collection/test_lb_data_holder.cc Outdated Show resolved Hide resolved
@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch 2 times, most recently from 325dcdb to b9fe835 Compare May 10, 2023 08:56
@thearusable thearusable requested a review from nlslatt May 10, 2023 08:59
@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch from b9fe835 to beaf3c8 Compare May 10, 2023 15:36
nlslatt
nlslatt approved these changes May 10, 2023
View reviewed changes
Copy link
Collaborator

@nlslatt nlslatt left a comment

Choose a reason for hiding this comment

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

Should all the checks in the tests really be ASSERT_* or should some be EXPECT_*? I recommend only using ASSERT_* when the checks that follow are no longer valid (and might segfault or do something else bad). Using EXPECT_* gives the opportunity to see what independent checks are failing later on in the test.

@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch from beaf3c8 to ea40f84 Compare May 16, 2023 12:29
@thearusable thearusable force-pushed the 2130-spec-for-skipped-phases-2 branch from ea40f84 to 526a6ed Compare May 16, 2023 12:34
@thearusable
Copy link
Contributor Author

@nlslatt Yes, they should be EXPECT_* instead of ASSERT_*. I left only a few ASSERT_*.

@nmm0 nmm0 mentioned this pull request May 16, 2023
Open
@nlslatt nlslatt merged commit 522945b into develop May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlslatt nlslatt nlslatt approved these changes

@lifflander lifflander lifflander approved these changes

@PhilMiller PhilMiller Awaiting requested review from PhilMiller

@stmcgovern stmcgovern Awaiting requested review from stmcgovern

@cz4rs cz4rs Awaiting requested review from cz4rs

@JacobDomagala JacobDomagala Awaiting requested review from JacobDomagala

Assignees

@thearusable thearusable

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add specification for LB data file for phases skipped or identical
3 participants
@thearusable @lifflander @nlslatt