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 System::project_solution/System::project_vector methods utilizing FEMFunctionBase functors #21

Merged
merged 5 commits into from Jan 31, 2013

Conversation

pbauman
Copy link
Member

@pbauman pbauman commented Dec 20, 2012

This adds the capability for the user to projection_solution using a FEMFunctionBase object. This required updating FEMFunctionBase to behave similarly to FunctionBase objects, including adding some pure virtual methods that will need to be implemented in any user codes (Vikram is the only other user of these that I'm currently aware of). The key difference in the FEMFunctionBase is the operator() methods will have access to a FEMContext object. The implementation of project_vector is very similar to the other methods, with a fair bit of copy/pasted code, but I use the FEMContext object that gets constructed to setup shape functions etc.

In short, the use case for this was for me to be able to create an output system and use a FEMFunctionBase object to project auxiliary quantities computed from my main multiphysics system; this is accomplished by caching a FEMContext from the multiphysics system in the FEMFunctionBase object.

I've updated adjoints_ex3 to implement pure virtual methods that were added, but there's no explicit use of these methods in the library yet - just my successful test in my application code. Any suggestions for adding to an existing example that would be quick? Otherwise, this tested cleanly with --enable-everything, MPI, TBB, and MPI+TBB. Did not try complex.

@buildhive
Copy link

libMesh - C++ Finite Element Library » libmesh #91 SUCCESS
This pull request looks good
(what's this?)

@@ -1711,6 +1819,627 @@ void ProjectSolution::operator()(const ConstElemRange &range) const
}


void ProjectFEMSolution::operator()(const ConstElemRange &range) const
Copy link
Member

Choose a reason for hiding this comment

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

This in particular looks like a lot of code duplication - can we do something clever with templates instead??

Similarly with the ProjectFEMSolution vs ProjectSolution classes - could their common functionality be in a base class or templated class?

Copy link
Member Author

Choose a reason for hiding this comment

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

On Fri, Dec 21, 2012 at 7:16 AM, Benjamin S. Kirk
notifications@github.comwrote:

This in particular looks like a lot of code duplication - can we do
something clever with templates instead??

Similarly with the ProjectFEMSolution vs ProjectSolution classes - could
their common functionality be in a base class or templated class?

It is quite a bit of code duplication. I'm not sure templating will work in
teh ProjectFEMSolution case because the algorithm is slightly different
because of the construction of the FEMContext object (in particular, the
element and variable loop order is switched to avoid superfluous element
reiniting). Nevertheless, I'll look into trying to remove some of the code
duplication.

However, at a higher level, I've been silently wondering whether we can
encapsulate these element-local solves better because very similar
algorithms appear in adaptivity, Dirichlet constraints, periodic
constraints, etc. In particular, HCURL/HDIV continuity has been on my TODO
list for awhile and part of that was figuring out all the different places
that would have to be put in. Would be nice to only have to do it once.

Copy link
Member

Choose a reason for hiding this comment

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

On Fri, 21 Dec 2012, Paul T. Bauman wrote:

I'm not sure templating will work in teh ProjectFEMSolution case
because the algorithm is slightly different because of the
construction of the FEMContext object (in particular, the element
and variable loop order is switched to avoid superfluous element
reiniting).

There shouldn't be any problem with switching the loop order in the
ordinary solution projection case.

Copy link
Member

Choose a reason for hiding this comment

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

I'll add to this discussion by saying that we have our own reimplementation of project_solution for many of the same reasons (efficiency and calling our own custom objects).

@buildhive
Copy link

libMesh - C++ Finite Element Library » libmesh #131 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

libMesh - C++ Finite Element Library » libmesh #133 SUCCESS
This pull request looks good
(what's this?)

@roystgnr
Copy link
Member

Is this passing your tests after the side/edge fixes? I don't see any other problems that would preclude merging.

@pbauman
Copy link
Member Author

pbauman commented Jan 22, 2013

It does appear to be functional now. However, I haven't yet addressed the code duplication issue. Do you still want me to take a look at that before merging this?

@roystgnr
Copy link
Member

On Tue, 22 Jan 2013, Paul T. Bauman wrote:

It does appear to be functional now. However, I haven't yet
addressed the code duplication issue. Do you still want me to take a
look at that before merging this?

Up to you. If you're starting to depend on this functionality in
GRINS then I'd say merge now and refactor later; if not then it
might be nice to refactor first.

@roystgnr
Copy link
Member

My data type commits probably completely hosed you here; let me know if you want me to update your stuff (and merge it if you're ready).

@buildhive
Copy link

libMesh - C++ Finite Element Library » libmesh #237 SUCCESS
This pull request looks good
(what's this?)

@pbauman
Copy link
Member Author

pbauman commented Jan 31, 2013

OK, I rebased off the current master. I think, given Roy's previous comments, I'd like to get this in now for the 0.9.0 release and open a separate issue to track the refactoring to get rid of the code duplication. Ben, any strong objections?

@benkirk
Copy link
Member

benkirk commented Jan 31, 2013

No strong issues. I'll bug you to refactor if and when this section of code needs an update. ;-)

benkirk added a commit that referenced this pull request Jan 31, 2013
Add System::project_solution/System::project_vector methods utilizing FEMFunctionBase functors
@benkirk benkirk merged commit 01901ac into libMesh:master Jan 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants