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

check number of active cells in shared::Tria #5362

Merged
merged 1 commit into from Nov 1, 2017

Conversation

tjhei
Copy link
Member

@tjhei tjhei commented Oct 30, 2017

part of #5359

@tjhei
Copy link
Member Author

tjhei commented Oct 30, 2017

note: tests are still running here.

AssertThrowMPI(ierr);

const unsigned int total_cells
= Utilities::MPI::sum(n_my_cells, this->get_communicator());
Copy link
Member

Choose a reason for hiding this comment

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

nice cleanup!

Copy link
Member

Choose a reason for hiding this comment

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

It seems a bit risky to me to use an unsigned int here for the global number of cells. Would you mind switching to std::size_t?

Copy link
Member

Choose a reason for hiding this comment

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

It's a shared triangulation, so the sum of the number of locally owned cells (n_my_cells) adds up to the number of active cells each processor stores itself locally. I think it's unlikely that we're ever going to have machines where a single MPI process will store more than 4B cells :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the type of n_active_cells is an unsigned int and is equal to total_cells.

Copy link
Member

Choose a reason for hiding this comment

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

good point; I forgot that all cells are stored locally in this case.

@bangerth
Copy link
Member

OK to merge once the tester is happy.

/run-tests

@tjhei
Copy link
Member Author

tjhei commented Oct 30, 2017

all shared tests run correctly here (unsurprisingly)

= Utilities::MPI::max(this->n_active_cells(), this->get_communicator());
Assert(max_active_cells == this->n_active_cells(),
ExcMessage("A shared::Triangulation needs to be refined in the same "
"way on all processors, but there are currently a different "
Copy link
Member

Choose a reason for hiding this comment

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

The phrasing sounds (grammatically) wrong. Maybe
[...], but the participating processors don't agree on the number of active cells.
or
[...], but there are currently different numbers of active cells on the participating processors.

Copy link
Member

Choose a reason for hiding this comment

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

I am also fine if you fix this up in a subsequent PR if you prefer that.

const unsigned int max_active_cells
= Utilities::MPI::max(this->n_active_cells(), this->get_communicator());
Assert(max_active_cells == this->n_active_cells(),
ExcMessage("A shared::Triangulation needs to be refined in the same "
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be parallel::shared::Triangulation

@tjhei
Copy link
Member Author

tjhei commented Oct 31, 2017

updated.

@kronbichler kronbichler merged commit 27735ff into dealii:master Nov 1, 2017
@tjhei tjhei deleted the shared_check_same branch November 1, 2017 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants