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

Add more python wrappers for GridTools #10875

Merged
merged 4 commits into from
Sep 15, 2020
Merged

Conversation

agrayver
Copy link
Contributor

@agrayver agrayver commented Sep 1, 2020

  • Replace existing merge_triangulations wrapper that accepts only two triangulations with a more generic equivalent.
  • Add python wrapper for GridTools::replicate_triangulation

Part of #9015

@agrayver agrayver changed the title Add python wrappers Add more python wrappers for GridTools Sep 1, 2020
GridGenerator::merge_triangulations(*tria_1, *tria_2, *tria);
// We need to reassign tria to triangulation because tria was cleared
// inside merge_triangulations.
triangulation = tria;
Copy link
Member

Choose a reason for hiding this comment

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

So this part is not necessary anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what happens inside

result.create_triangulation(vertices, cells, subcell_data);

Gigen that we pass the argument by reference, I don't see a need for this. Am I missing something?

more general version of the GridTools::merge_triangulations is
implemented
<br>
(Alexander Grayver, 2020/09/01)
Copy link
Member

Choose a reason for hiding this comment

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

You technically also introduce an incompatibility in merge_triangulation So can you add an entry there.

@Rombur
Copy link
Member

Rombur commented Sep 3, 2020

/rebuild

@agrayver
Copy link
Contributor Author

agrayver commented Sep 3, 2020

Thanks, @Rombur! I added an incompatibility note. Also, I added two optional parameters of the original C++ function to the python wrapper.

Copy link
Member

@Rombur Rombur left a comment

Choose a reason for hiding this comment

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

This looks good but since the tester is failing can you rebase the PR on master to see if it fixes the problem.

@agrayver
Copy link
Contributor Author

agrayver commented Sep 15, 2020

@Rombur should be good now.

@Rombur Rombur merged commit db91862 into dealii:master Sep 15, 2020
@agrayver agrayver deleted the python_wrappers branch September 16, 2020 06:24
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

2 participants