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

Remove hp::DoFHandler::get_fe() #9734

Merged
merged 1 commit into from Mar 27, 2020
Merged

Remove hp::DoFHandler::get_fe() #9734

merged 1 commit into from Mar 27, 2020

Conversation

peterrum
Copy link
Member

This PR removes the deprecated method hp::DoFHandler::get_fe(). I guess we can do this since the function has been deprecated more than two years ago.

Do you agree @bangerth and @marcfehling?

@kronbichler
Copy link
Member

/rebuild

bangerth
bangerth reviewed Mar 25, 2020
View reviewed changes
include/deal.II/numerics/vector_tools.templates.h Outdated
@@ -277,16 +277,21 @@ namespace VectorTools
int spacedim,
typename VectorType,
template <int, int> class DoFHandlerType,
typename T>
typename T,
typename F>
Copy link
Member

Choose a reason for hiding this comment

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

can you give this argument a more descriptive template type name? Maybe FEorFECollectionType?

Copy link
Member

Choose a reason for hiding this comment

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

On the other hand, why do you need this? Regular DoFHandler has a function get_fe_collection() so that's a function you can generically call...

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 Right! I was happy that after pages of compiler errors it compiled ;)

masterleinad
masterleinad reviewed Mar 25, 2020
View reviewed changes
include/deal.II/matrix_free/matrix_free.templates.h Outdated
@@ -529,7 +529,7 @@ MatrixFree<dim, Number, VectorizedArrayType>::internal_reinit(
{
unsigned int n_components = 0;
for (unsigned int no = 0; no < dof_handler.size(); ++no)
n_components += dof_handler[no]->get_fe()[0].n_base_elements();
n_components += dof_handler[no]->get_fe_collection()[0].n_base_elements();
Copy link
Member

Choose a reason for hiding this comment

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

I would rather use

Suggested change
n_components += dof_handler[no]->get_fe_collection()[0].n_base_elements();
n_components += dof_handler[no]->get_fe(0).n_base_elements();

here and at the other places similarly.

@peterrum peterrum force-pushed the hp_dofhandler_remove_get_fe branch from be2a128 to 0fb6fcc Compare March 26, 2020 00:05
@peterrum
Copy link
Member Author

@bangerth @masterleinad Thanks for the review! I have made the changes.

@masterleinad
Copy link
Member

Did you actaully remove the member function?

marcfehling
marcfehling approved these changes Mar 26, 2020
View reviewed changes
Copy link
Member

@marcfehling marcfehling left a comment

Choose a reason for hiding this comment

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

Looks good! Found nothing major. I think you don't have to introduce the two interpolate functions.
I agree with @masterleinad: Did you remove the get_fe() function?

doc/news/changes/incompatibilities/20200326PeterMunch Outdated
@@ -0,0 +1,4 @@
Removed: The depricated method hp::DoFHandler::get_fe() has been removed. Please use
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Removed: The depricated method hp::DoFHandler::get_fe() has been removed. Please use
Removed: The deprecated method hp::DoFHandler::get_fe() has been removed. Please use

include/deal.II/numerics/vector_tools.templates.h
@@ -553,21 +607,7 @@ namespace VectorTools
VectorType & vec,
const ComponentMask & component_mask)
{
Assert(dof_handler.get_fe().n_components() == function.n_components,
Copy link
Member

Choose a reason for hiding this comment

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

Do you actually need to introduce your new two internal::interpolate(...) functions? Both DoFHandler and hp::DoFHandler have the member function get_fe_collection().

I guess if you just replace dof_handler.get_fe() with dof_handler.get_fe_collection() in this assertion, you should be fine!

bangerth
bangerth requested changes Mar 26, 2020
View reviewed changes
include/deal.II/numerics/vector_tools.templates.h
@@ -553,21 +607,7 @@ namespace VectorTools
VectorType & vec,
const ComponentMask & component_mask)
{
Assert(dof_handler.get_fe().n_components() == function.n_components,
ExcDimensionMismatch(dof_handler.get_fe().n_components(),
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand why you need to duplicate this function into one for the hp and one for the non-hp case. Can't you just replace this call to get_fe() by get_fe_collection()? That works for both ::DoFHandler and hp::DoFHandler. If you did that, then you wouldn't have to duplicate the body of the function...

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@peterrum peterrum force-pushed the hp_dofhandler_remove_get_fe branch from 0fb6fcc to 2303887 Compare March 26, 2020 07:05
@peterrum
Copy link
Member Author

Did you actaully remove the member function?

Oops! Not it is removed. I removed the function on an other branch, where I am working of DoFHandler and hp::DoFHandler.

@peterrum peterrum force-pushed the hp_dofhandler_remove_get_fe branch from 2303887 to 02c5c34 Compare March 26, 2020 13:48
@masterleinad
Copy link
Member

@bangerth still needs to approve.

bangerth
bangerth approved these changes Mar 26, 2020
View reviewed changes
Copy link
Member

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

Yes, better! :-)

@masterleinad
Copy link
Member

Seems like you missed one test:

/jenkins/workspace/dealii_PR-9734/tests/dofs/dof_tools_12.cc:31:68: error: no matching function for call to ‘dealii::hp::DoFHandler<1, 1>::get_fe() const’
   std::vector<bool> mask(dof_handler.get_fe().n_components(), false);

@peterrum peterrum force-pushed the hp_dofhandler_remove_get_fe branch from 02c5c34 to 9765fe6 Compare March 26, 2020 23:51
@masterleinad masterleinad merged commit 05c59b5 into dealii:master Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@masterleinad masterleinad masterleinad approved these changes

@bangerth bangerth bangerth approved these changes

@kronbichler kronbichler kronbichler approved these changes

@marcfehling marcfehling marcfehling approved these changes

Assignees
No one assigned
Labels
DoFs 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
5 participants
@peterrum @kronbichler @masterleinad @bangerth @marcfehling