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.

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

CommunicationPlan copy #295

Merged
merged 7 commits into from
Sep 22, 2020
Merged

CommunicationPlan copy #295

merged 7 commits into from
Sep 22, 2020

Conversation

streeve
Copy link
Member

@streeve streeve commented Sep 11, 2020

First, adding a test which fails in copying the CommunicationPlan due to duplicating communicators. Add missing copy constructor/assignment operator.

Found during periodic communication #263 with @sslattery

Increased coverage by explicitly setting MPI_EXEC_NUMPROCS=2 for travis (previously running everything one one rank)

@streeve streeve added the bug Something isn't working label Sep 11, 2020
@streeve streeve self-assigned this Sep 11, 2020
@codecov
Copy link

codecov bot commented Sep 11, 2020

Codecov Report

Merging #295 into master will increase coverage by 1.8%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #295     +/-   ##
========================================
+ Coverage    93.4%   95.2%   +1.8%     
========================================
  Files          50      50             
  Lines        2944    2952      +8     
========================================
+ Hits         2750    2811     +61     
+ Misses        194     141     -53     
Flag Coverage Δ
#clang 95.2% <100.0%> (+1.8%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
core/src/Cabana_CommunicationPlan.hpp 98.6% <100.0%> (+8.3%) ⬆️
cajita/src/Cajita_Halo.hpp 98.1% <0.0%> (+0.9%) ⬆️
cajita/src/Cajita_GlobalGrid.cpp 100.0% <0.0%> (+6.5%) ⬆️
core/src/Cabana_Distributor.hpp 95.5% <0.0%> (+18.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 783faf4...626377c. Read the comment docs.

@streeve
Copy link
Member Author

streeve commented Sep 18, 2020

Updated to match arborx/ArborX#369

@streeve streeve changed the title CommunicationPlan copy constructor CommunicationPlan copy Sep 18, 2020
@streeve
Copy link
Member Author

streeve commented Sep 18, 2020

It looks like the codecov patch is failing because we're only running with one MPI rank on travis (and therefore not testing a reasonable fraction of the CommPlan). @junghans is there an easy way to get this to run on at least 2 MPI ranks?

@junghans
Copy link
Member

Try to set MPIEXEC_MAX_NUMPROCS=2.

@streeve
Copy link
Member Author

streeve commented Sep 21, 2020

Try to set MPIEXEC_MAX_NUMPROCS=2.

@junghans I added to Travis, but it didn't seem to change anything

@junghans
Copy link
Member

Try to set MPIEXEC_MAX_NUMPROCS=2.

@junghans I added to Travis, but it didn't seem to change anything

fixed

@streeve
Copy link
Member Author

streeve commented Sep 21, 2020

@sslattery this is ready for re-review

@sslattery sslattery merged commit 9ba3b6d into ECP-copa:master Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants