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

Generalize DoFHandlerPolicy of p:d:t #8579

Merged
merged 1 commit into from Aug 20, 2019
Merged

Generalize DoFHandlerPolicy of p:d:t #8579

merged 1 commit into from Aug 20, 2019

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented Aug 15, 2019 •
edited

This PR is part of the effort to introduce the new parallel::fullydistributed::Triangulation (see #8558) and is a follow-up to PR #8586.

In this PR, I eliminate the direct usage of coarse_cell_to_p4est_tree_permutation and p4est_tree_to_coarse_cell_permutation int the DoFHandlerPolicy of p:d:t. The translation is moved inside of translate_coarse_cell_id_to_coarse_cell_index and translate_coarse_cell_index_to_coarse_cell_id of p:d:t, since it is a p:d:t-specific thing and is independent from the policy.

Ping: @bangerth @tjhei

@peterrum
Copy link
Member Author

peterrum commented Aug 15, 2019 •
edited

This PR is totally WIP and dependents on the merge of PR #8567. It should make clear the implications of PR #8567 (i.e. only the diff is interesting).

@peterrum peterrum force-pushed the pdt_dofhandlerpolicy_generalization branch 4 times, most recently from fc58308 to 664a515 Compare August 16, 2019 00:41
@peterrum
Copy link
Member Author

@masterleinad Could I ask you for a favor? Could you add ready to test-label and /rebuild (and do not merge-label, since PR #8567 first has to merged) to this PR. Local tests show that the restructuring of the DoFHandlerPolicy seems to work, however I have not the possibility at the moment to run all tests.

@peterrum peterrum force-pushed the pdt_dofhandlerpolicy_generalization branch from 664a515 to 75b2042 Compare August 16, 2019 03:28
@masterleinad
Copy link
Member

The MPI tester doesn't run many tests at all. Locally, the following mpi tests are failing for me:

     7377 - mpi/mg_ghost_dofs_periodic_06.mpirun=7.debug (Failed)
     7378 - mpi/mg_ghost_dofs_periodic_06.mpirun=7.release (Failed)
     9275 - sharedtria/dof_05.mpirun=3.debug (Failed)

@peterrum peterrum force-pushed the pdt_dofhandlerpolicy_generalization branch from 75b2042 to 660234c Compare August 16, 2019 04:45
@peterrum
Copy link
Member Author

@masterleinad Thanks a lot! I had a look at the failing tests. These are tests directly messing with CellId. I have updated them and now they should work!

@peterrum peterrum mentioned this pull request Aug 16, 2019
Merged
kronbichler
kronbichler reviewed Aug 16, 2019
View reviewed changes
doc/news/changes/minor/20190814PeterMunch-1 Outdated
@@ -0,0 +1,4 @@
New: Generalize DoFHandlerPolicy of p:d:t such that it uses
Copy link
Member

Choose a reason for hiding this comment

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

Please spell out the names properly to have doxygen pick them up.

bangerth
bangerth requested changes Aug 16, 2019
View reviewed changes
doc/news/changes/minor/20190815PeterMunch Outdated
@@ -0,0 +1,3 @@
New: Add method to get the coarse-grid cell from CellID.
<br>
(Peter Munch, 2019/08/15)
Copy link
Member

Choose a reason for hiding this comment

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

You want to drop this commit from this pull request.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bangerth I have drop it.

@peterrum peterrum force-pushed the pdt_dofhandlerpolicy_generalization branch from 660234c to 25410d2 Compare August 17, 2019 02:12
@peterrum peterrum mentioned this pull request Aug 17, 2019
Closed
11 tasks
@peterrum peterrum force-pushed the pdt_dofhandlerpolicy_generalization branch from 25410d2 to d8ddd6b Compare August 17, 2019 23:10
bangerth
bangerth reviewed Aug 18, 2019
View reviewed changes
include/deal.II/distributed/tria.h Outdated
@@ -1217,6 +1217,16 @@ namespace parallel
std::vector<bool>
mark_locally_active_vertices_on_level(const int level) const;

virtual unsigned int
translate_coarse_cell_id_to_coarse_cell_index(
const unsigned int coarse_cell_id) const override;
Copy link
Member

Choose a reason for hiding this comment

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

Didn't you rename this in some other PR to drop the translate_ prefix? I actually like the prefix, but I think @tjhei didn't.

You'll also want to use the new data type for coarse cell ids here.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bangerth I also prefer the translate_ prefix because it indicates that it is a method. However, I have made this PR now consistent with the previous PRs.

Copy link
Member

Choose a reason for hiding this comment

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

Let's go with this then. I wouldn't mind if you had a follow-up PR that changes the name back again. It should be easy to do with find-replace.

@peterrum peterrum force-pushed the pdt_dofhandlerpolicy_generalization branch from d8ddd6b to 2bea866 Compare August 18, 2019 20:04
@bangerth
Copy link
Member

/rebuild

@masterleinad
Copy link
Member

It seems that you either need to rebase or add a missing header.

@peterrum
Copy link
Member Author

@masterleinad: You are right. This PR will compile once #8567 and #8586 have been merged, and I have rebased this branch.

@masterleinad
Copy link
Member

I added the Do not merge label for now.

@peterrum peterrum force-pushed the pdt_dofhandlerpolicy_generalization branch from 2bea866 to e764517 Compare August 20, 2019 06:04
@peterrum
Copy link
Member Author

@tjhei @masterleinad @bangerth I have rebased this branch!

@masterleinad
Copy link
Member

Great!

@masterleinad masterleinad merged commit 7160649 into dealii:master Aug 20, 2019
@peterrum peterrum mentioned this pull request Aug 25, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bangerth bangerth bangerth approved these changes

@kronbichler kronbichler kronbichler left review comments

Assignees
No one assigned
Labels
Do not merge ☠️ ready to test Reviewed and ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@peterrum @masterleinad @bangerth @kronbichler