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

[WIP] Make geometric calculations more generic #2396

Closed
wants to merge 5 commits into from

Conversation

lindsayad
Copy link
Member

@lindsayad lindsayad commented Jan 6, 2020

Classes that need to be templated:

  • FE*
  • Node
  • Point
  • Elem*
  • Mesh*

Other To-Do items:

  • Compile
  • Unit tests
  • Fix merge conflicts
  • Have someone review/approve this monstrosity

We'll have to talk about what we want to do for backwards compatability.

Closes #2121

@roystgnr
Copy link
Member

roystgnr commented Jan 6, 2020

"Work in progress" means I can merge #2395 and undercut all your hard work without feeling too guilty?

@lindsayad
Copy link
Member Author

Sigh...haha yes go ahead.

@roystgnr
Copy link
Member

roystgnr commented Jan 6, 2020

Ouch, I barely changed browser tabs before github started yelling at us here about conflicting files.

Sorry about the hassle. On the bright side, the expanded unit test coverage should make it easier to do this work without regressions!

@roystgnr
Copy link
Member

roystgnr commented Jan 7, 2020

@jwpeterson pointed out to me that our _impl.h convention (where most users don't need to include them because we instantiate the common case(s) manually) isn't intuitively obvious. Let's come up with some boilerplate comment to document that convention in the _impl.h headers as part of this PR, then when we're happy with that we can add it to the existing dense_matrix*_impl.h headers too?

@lindsayad
Copy link
Member Author

Sounds good to me

@lindsayad lindsayad force-pushed the template-fe-map branch 4 times, most recently from db77e13 to 4305ca5 Compare January 8, 2020 02:53
@jwpeterson
Copy link
Member

So @lindsayad I think you are going to have to template every class in libmesh, like EquationSystems, System, etc.? I'm starting to get scared.

@lindsayad
Copy link
Member Author

So @lindsayad I think you are going to have to template every class in libmesh, like EquationSystems, System, etc.? I'm starting to get scared.

Haha, this is why I threw this up here (with a WIP label) so that you guys could give your input as I go. So I think that I should only have to class template geometric entities. Algebraic entities should escape relatively unscathed, except that algebraic class methods which include geometric entities in their argument list will probably have to become templated; you can see d2b9728 for an example of this. If an algebraic class has a geometric entity as a data member or a method with a geometric return type that is unrelated to the argument list (e.g. I wouldn't be able to deduce a function template argument from the function args), then I have to get a little more clever. So for the DofMap for example, I ended up creating a MeshGhosting class which MeshBase inherits from that contains the APIs that the DofMap needs to use and then changed the DofMap::_mesh data member to be of MeshGhosting type.

I've been thinking about how to do this with any less work (and any less destruction) than what I've been doing but so far I've yet to have any bright ideas.

@lindsayad
Copy link
Member Author

I never thought I'd see the day...my branch compiles!!! Now of course compiling a RealType = DualNumber unit test...that will be the next battle

@lindsayad lindsayad force-pushed the template-fe-map branch 2 times, most recently from 1eef47e to 730fb93 Compare February 4, 2020 01:00
@lindsayad
Copy link
Member Author

lindsayad commented Feb 4, 2020

Has anyone ever seen an error like this before?

In file included from ../include/libmesh/dof_map_impl.h:40,
from ../include/libmesh/instantiate_real_type.h:22,
from /home/lindad/projects/moose/scripts/../libmesh/tests/ad/geometric_ad_test.C:2:
/home/lindad/projects/moose/scripts/../libmesh/contrib/timpi/src/algorithms/include/timpi/parallel_sync.h:502:6: error: ‘void TIMPI::push_parallel_vector_data(const TIMPI::Communicator&, const MapType<unsigned int, std::__debug::vector<std::__debug::vector<ValueType, A1>, A2>, ExtraTypes ...>&, const ActionFunctor&) [with MapType = std::__debug::map; ValueType = std::pair<unsigned int, double>; A1 = std::allocator<std::pair<unsigned int, double> >; A2 = std::allocator<std::__debug::vector<std::pair<unsigned int, double> > >; ExtraTypes = {std::less, std::allocator<std::pair<const unsigned int, std::__debug::vector<std::__debug::vector<std::pair<unsigned int, double>, std::allocator<std::pair<unsigned int, double> > >, std::allocator<std::__debug::vector<std::pair<unsigned int, double>, std::allocator<std::pair<unsigned int, double> > > > > > >}; ActionFunctor = libMesh::DofMap::allgather_recursive_constraints(libMesh::MeshBaseTempl&) [with RealType = MetaPhysicL::DualNumber<double, MetaPhysicL::DynamicSparseNumberArray<double, unsigned int> >]::<lambda(libMesh::processor_id_type, const std::__debug::vector<std::__debug::vector<std::pair<unsigned int, double> > >&)>]’, declared using local type ‘const libMesh::DofMap::allgather_recursive_constraints(libMesh::MeshBaseTempl&) [with RealType = MetaPhysicL::DualNumber<double, MetaPhysicL::DynamicSparseNumberArray<double, unsigned int> >]::<lambda(libMesh::processor_id_type, const std::__debug::vector<std::__debug::vector<std::pair<unsigned int, double> > >&)>’, is used but never defined [-fpermissive]

Used but never defined...I don't get it. There's clearly a definition for the template TIMPI::push_parallel_vector_data and you can't declare a lambda without a definition...

@lindsayad
Copy link
Member Author

lindsayad commented Feb 4, 2020

I'm seeing two paths forward with this:

  1. Don't template algebraic classes at all. This would essentially necessitate the user always having two meshes (which is what MOOSE does) and the algebraic objects (EquationSystems, System, DofMap) would always interact with the non-displaced mesh. This would lead to a lot less content in this PR, but would make it rather inconvenient to query the displaced mesh's elements and nodes for dof information.
  2. Basically template the whole world, e.g. template EquationSystems, System, DofMap. I don't think it makes sense to just say template EquationSystems because then System would have to get_equation_systems().get_mesh() in order to get a complete mesh object (e.g. not some dumbed-down base class object that you can't query for elements/nodes etc.)

I'm leaning towards 2 because it is more consistent with the current conceptual libMesh model which is that a mesh is paired with an EquationSystems object and its associated Systems/DofMaps. But this will be more disruptive to the libMesh code base. No matter what, I don't think we're going to want to merge this into master for a while...I'm doing all this work so that we can get more accurate/perfect Jacobians for displaced problems, but I'm not even sure if we're going to see performance gains, e.g. will a decreased non-linear iteration count offset the computational cost of sparsity computations in MetaPhysicL types? Moreover, for this to even be feasible for a large problem, we're going to have to use a dynamic derivative container type for memory reasons. And we've already seen that these containers are relatively slow (see roystgnr/MetaPhysicL#1). I (and @fdkong and @friedmud) believe we can fix this with a memory pool. That's going to be even more development time.

In summary, I think I'm going to need to:

  1. "Complete" the content of this PR
  2. Develop the memory pool based derivative container type in MetaPhysicL
  3. In a branch update MOOSE to interface with the content of this PR
  4. Run a significant amount of unit tests and legitimate physical problems in MOOSE applications
  5. Based on those results, determine whether it's worth it to overhaul libMesh with this PR

@lindsayad
Copy link
Member Author

On second thought, I'm leaning towards not templating algebraic objects at all. You know, Occam's Razor and stuff. Then I'd just have to do something smart for cloning DofObject information on to the displaced mesh's Nodes and Elems

@roystgnr
Copy link
Member

roystgnr commented Feb 4, 2020

So that "used but never defined" looks like my bug? At one point I had a KeyType parameter in the push vector specialization, but now it looks like the KeyType parameter exists in the declaration but not in the definition, and so your compiler may rightly be confused by that. Not sure why my compiler and the CI compilers haven't already hit that or how you set it off, though.

@roystgnr
Copy link
Member

roystgnr commented Feb 4, 2020

There's got to be a better phrase than "algebraic classes" for System/DofMap/EquationSystems ... if only because I've already abused that vocabulary in the opposite direction in parallel_algebra.h ... let me call them "system classes" for lack of a better word.

I agree with your dichotomy, and my vote would be to avoid templating the system classes if possible, and I think it would be possible, if we're willing to allow for a little dynamic_cast hackery in user space to get AD geometry when that's available.

@lindsayad
Copy link
Member Author

There's got to be a better phrase than "algebraic classes" for System/DofMap/EquationSystems

Seemed consistent with algebraic ghosting 😅

I agree with your dichotomy, and my vote would be to avoid templating the system classes if possible, and I think it would be possible, if we're willing to allow for a little dynamic_cast hackery in user space to get AD geometry when that's available.

Cool 👍

@roystgnr
Copy link
Member

roystgnr commented Feb 5, 2020

Seemed consistent with algebraic ghosting 😅

Oh, well, see, there's your problem. If you try to come up with your own names then consistency is difficult, but if you try to match my naming schemes then consistency is impossible.

lindsayad added a commit to lindsayad/libmesh that referenced this pull request Feb 13, 2020
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
lindsayad added a commit to lindsayad/libmesh that referenced this pull request Feb 19, 2020
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
lindsayad added a commit to lindsayad/libmesh that referenced this pull request Feb 25, 2020
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
lindsayad added a commit to lindsayad/libmesh that referenced this pull request Mar 27, 2020
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
lindsayad added a commit to lindsayad/libmesh that referenced this pull request May 12, 2020
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
@lindsayad lindsayad mentioned this pull request Dec 1, 2020
@jwpeterson
Copy link
Member

This PR has been marked "do not merge" since we are no longer accepting PRs into the master branch. All new PRs should be made on the devel branch instead. Once this PR's target branch has been updated to devel, the "do not merge" label will be removed.

lindsayad added a commit to lindsayad/libmesh that referenced this pull request Jan 3, 2023
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
lindsayad added a commit to lindsayad/libmesh that referenced this pull request Jan 10, 2023
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
lindsayad added a commit to lindsayad/libmesh that referenced this pull request Jan 10, 2023
This is a much simpler in terms of lines of code (albeit perhaps slower) implementation of
the goal of realizing geometric and finite element basis evaluations
with derivative information, which has its usefulness in displaced
mesh calculations.

Refs libMesh#2121 libMesh#2396
@lindsayad
Copy link
Member Author

Closing in favor of the research work on #2576

@lindsayad lindsayad closed this Feb 4, 2023
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.

Automatic Differentiation for geometric entities?
3 participants