DofManager #271
DofManager #271
Conversation
joshua-white
requested review from
andrea-franceschini,
rrsettgast and
AntoineMazuyer
|
Okay, I will start working on the basic implementation right now, since there is some obvious infrastructure to set up. Let's then plan to meet Tuesday at Stanford to discuss the desired design and we can go from there. |
|
|
||
| blt_add_test( NAME ${test_name} | ||
| COMMAND ${test_name} | ||
| NUM_MPI_TASKS 2 ) | ||
| COMMAND ${test_name} -i ${CMAKE_CURRENT_LIST_DIR}/testDofManager.xml -x ${nranks} |
There was a problem hiding this comment.
Is this going to be ignored by the other tests?
There was a problem hiding this comment.
For now, all tests read testDofManager.xml but then ignore it if they don't need it. I'll make them separate so they can have different commands.
As a separate note, I had to copy quite a bit of GEOSX main() code to a test main() in order to set up a problem manager / mesh that could then be used to test the DofManager. It looks like Sergey did a similar thing for the compositional solver. Is it worth trying to create some reusable code somewhere for the test suite that would set up a relatively complete GEOSX "environment" for subsequent testing (i.e. initialize MPI comms, problem manager, input parsing). Does that make sense? I imagine people will be doing this all over the test suite. At some point the "unit" tests start to look pretty integrated.
There was a problem hiding this comment.
yes it does. However, the lines between unit test and integrated test get a little blurry. Perhaps put a dummy data structure function in ProblemManager.
|
Hi All, I uploaded a quick prototype for the DofManager. Please see DofManager.hpp. Think of this as a general proposal for the interface rather than working code. |
| * DoFManager and the LAI implementation, without relying on a specific | ||
| * LAI choice | ||
| */ | ||
| struct SparsityPattern |
There was a problem hiding this comment.
I would like to use a similar structure for FVManager to store FV stencils (but make it a weighted graph, and also a full-fledged class, with interfaces for incremental construction, etc.). Do we want to take advantage of the functionality overlap and define a generic templated CSRGraph class? Or would you rather keep them separate?
There was a problem hiding this comment.
I've gone back and forth a few times about the best way to handle graphs. We have many options:
- The lightweight option here, which doesn't provide any functionality at all. Its just a simple way to pass sparsity pattern information back and forth between different code components. I started here to get the code up and running asap. It's not a good endpoint.
- Writing our own CRSGraph as you suggested, with more complete functionality.
- Wrapping a graph implementation from somewhere else (Trilinos, Hypre, PETSc, ParMetis).
For your case, a weighted graph is basically a CRSMatrix, right? Can we use one of our wrapped ParallelMatrix objects for this purpose?
There was a problem hiding this comment.
I think the sparsity pattern has merit on its own. The stencil (FVM) and stiffness matrix (FEM) are essentially copies of the sparsity pattern, but with double values rather than indices. Is this the right way to think about it?
There was a problem hiding this comment.
I agree with Randy. Once the sparsity pattern is done, to create the stencil and stiffness matrix is just a matter of assembling double values in the right positions.
There was a problem hiding this comment.
we can create a
template< typename T, int NDIM >
class csrArray;
as part of the array classes to handle all csrArray-like needs.
| * Example 2 = add("pressure",CELL,FACE,1) for a scalar TPFA-type approximation | ||
| * Example 3 = add("pressure",CELL,NODE,1) for a scalar MPFA-type approximation | ||
| * Example 4 = add("mass",CELL,NONE,1) for a diagonal-only sparsity pattern (no connections) | ||
| * |
There was a problem hiding this comment.
If we have additional "Control volumes" for faults and fractures, what is the best way to proceed ? Because we will have, if the fracture is represented as a surface :
- Faces connected to cells through.... faces I guess ?
- Faces connected to faces through edges.
(There is also a similar issue with the wells.)
There was a problem hiding this comment.
The other complicated cases to consider are: (1) when physics is only assigned to certain element regions, but not others, and (2) a multilevel extension of the DofManager. I thought I would tackle the obvious cases first before struggling through the harder ones. The functionality and basic operations will always be the same (assigning dofs, using connectivity lists to compute sparsity patterns, etc) but the input specification could get more complicated.
There was a problem hiding this comment.
I think that this approach is fine for the DOF that correlate to some mesh pattern. For some of the non-mesh related connectivity, I think that the best/most flexible approach will have to be a manual input.
for 1) there could be some indicator of element region in the interface. Since the regions only apply to elements, the faces and nodes won't be dependent on the region....but if we ever decided to add the ability to have separate mesh bodies and treat each one as a separate region, then the fields will be useful. Perhaps instead of NODE, FACE, CELL there should be a path that is extracted from the input file. For instance if there is a mesh body called BODY0, and a region called REGION0, the input would be BODY0/NODE, and BODY0/FACE, and BODY0/REGION0/CELL
for 2) I think that there would be some benefit treating each level independently from the DOF manager perspective, but have a restriction and prolongation operator generated by the solver.
| /** | ||
| * Return global number of dofs across all processors. If field argument is "all", return monolithic size. | ||
| */ | ||
| globalIndex numGlobalDofs( string const & field = "all" ) const; |
There was a problem hiding this comment.
Concerning the MPI partitioning, we're probably going to deal with "Ghost" dofs ?
There was a problem hiding this comment.
Yes, though the GEOSX ghosted mesh and the linear algebra libraries can help us out a lot here.
There was a problem hiding this comment.
It will be just like the current implementation which does a synchronization of globalDOF numbers for each object that has a DOF.
|
Andrea. Do you need some assistance with a bug? It looks like you have a lot of commits trying to fix an apple error. Let me know if I can help. |
|
Yes!!! |
|
@AF1990 OK. Don't waste anymore time on it. I'll take care of it....if it is something that doesn't require major changes. |
|
Ok, thanks! No, it should not require major changes. It is a very localized problem. |
|
@AF1990 this is actually a really hard thing to diagnose. Valgrind gives some non-helpful information about a stack error. I don't know if you are using a ton of stack memory? Are you using some interdependent singletons? Playing around with all these commits, the only commit that doesn't fail is |
|
I know ... I already tried Valgring without success. Moreover, I don't use any stack memory. |
|
Finally I was able to understand the Apple XCode10 error!!! |
|
Can it be merged now??? |
|
Well, that's frustrating. But yes, let's merge. |
Yes ... quite frustrating ... |
|
Note that in the referenced issue, the solution was to make some static variables non-static, rather than changing the build order. While I'm happy this issue is fixed for now, it looks like we may have a hidden problem with static init/deinit order dependency? Are we sure it won't blow up elsewhere? Or is it purely an Apple bug that needs to be worked around? |
|
@klevzoff, you are right! But I don't use any static variable, except for the MAX_NUM_FIELD definition. |
|
@AF1990 I don't think the problem, if any, is with your code. I think it might just have surfaced in this PR due to a particular test and build sequence that appeared here. I actually think this is a bug with |
|
👍 |
|
So what is the static variable/object that is causing the problem? |
|
I really don't know. I checked and in the DofManager there are no static variables (except for a static member in the main class). As @klevzoff pointed out, probably, this a |
|
@rrsettgast I propose we merge this and just make a separate issue. I don't want to hold up progress on the DoFManager, and this issue seems to be a bit separate from that topic. There is something we don't understand about static variable treatment in Apple/Clang10. Changing the order in the linking phase having an impact is odd, but this is probably reflecting a problem elsewhere, not the DofManager. |
|
@joshua-white totally agree. do the honors.... |
Create a degree-of-freedom manager to provide convenient interface between physics solvers and linear algebra objects. This first cut at the implementation will focus on providing global indexing and sparsity pattern capabilities.