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

Virtual Element Method #1220

Merged
merged 32 commits into from
Jan 19, 2021
Merged

Virtual Element Method #1220

merged 32 commits into from
Jan 19, 2021

Conversation

andrea-borio
Copy link
Contributor

First hypothetical structure for VEM classes.

Also added a test to check build.
…ementBase.cpp

The generation of the test cube is now done succefully.
…rojectors.

Add computation of rotated points and diameter.
Add calls to get the required geometrical quantities.
…rojectors.

Add computation of integrals of 2D monomials on the boundary of the face.
The method now fills a vector containing the integral mean of projections of basis functions whose
support intersects the face.
Bug-fixes and changes to face computations.
Add computation of integral mean of derivatives of basis functions.
Add computation of the stabilization matrix.
Add computation of the number of quadrature points.
Code cleanups.
Fixed bugfix in computation of basisTimesMonomNormalDerBoundaryInt in
ConformingVirtualElementOrder1::ComputeProjectors(). Changed inputs of
ComputeFaceIntegrals() consequently.

All properties of ConformingVirtualElementOrder1 set to public to
allow easier testing. To be changed later, when getters are defined.
Face rotation matrices are now computed on-the-fly using pre-computed face normals.
@andrea-borio andrea-borio marked this pull request as ready for review December 10, 2020 11:30
@andrea-borio
Copy link
Contributor Author

This is the document with the pseudo-code of the implementation.

main.pdf

Copy link
Contributor

@francoishamon francoishamon left a comment

Choose a reason for hiding this comment

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

Thank you very much @andrea-borio , the PR looks great! I just made some small comments, but most of them can be addressed in the next PR if that's easier.

Comment on lines 32 to 36
localIndex numQuadraturePoints;
localIndex numSupportPoints;
array1d< real64 > basisFunctionsIntegralMean;
array2d< real64 > basisDerivativesIntegralMean;
array2d< real64 > stabilizationMatrix;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
localIndex numQuadraturePoints;
localIndex numSupportPoints;
array1d< real64 > basisFunctionsIntegralMean;
array2d< real64 > basisDerivativesIntegralMean;
array2d< real64 > stabilizationMatrix;
localIndex m_numQuadraturePoints;
localIndex m_numSupportPoints;
array1d< real64 > m_basisFunctionsIntegralMean;
array2d< real64 > m_basisDerivativesIntegralMean;
array2d< real64 > m_stabilizationMatrix;

Do they need to be public? Can they be protected instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you François! These properties should be protected for sure. They are public now for testing and because we still need to agree on suitable getters.


// Compute other geometrical properties.
// - compute rotation matrix.
array2d< real64 > faceRotationMatrix( 3, 3 );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
array2d< real64 > faceRotationMatrix( 3, 3 );
real64 faceRotationMatrix[ 3 ][ 3 ] = {{ 0.0 }};

Comment on lines +341 to +344
array1d< real64 > monomInternalIntegrals( 2 );
threeDMonomialIntegrals.resize( 3 );
LvArray::tensorOps::fill< 2 >( monomInternalIntegrals, 0.0 );
LvArray::tensorOps::fill< 3 >( threeDMonomialIntegrals, 0.0 );
Copy link
Contributor

Choose a reason for hiding this comment

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

Here to avoid resizing and filling the vectors with zero you could just do something more concise I think:

real64 monomInternalIntegrals[ 2 ] = { 0.0 };

and define in the other function threeDMonomialIntegrals as

real64 threeDMonomialIntegrals[ 3 ] = { 0.0 };

Comment on lines +44 to +60
// - compute cell diameter.
real64 cellDiameter = 0;
for( localIndex numVertex = 0; numVertex < numCellPoints; ++numVertex )
{
for( localIndex numOthVertex = 0; numOthVertex < numVertex; ++numOthVertex )
{
array1d< real64 > vertDiff( 3 );
LvArray::tensorOps::copy< 3 >( vertDiff, nodesCoords[cellToNodes( cellIndex, numVertex )] );
LvArray::tensorOps::subtract< 3 >( vertDiff,
nodesCoords[cellToNodes( cellIndex, numOthVertex )] );
real64 const candidateDiameter = LvArray::tensorOps::l2NormSquared< 3 >( vertDiff );
if( cellDiameter < candidateDiameter )
cellDiameter = candidateDiameter;
}
}
cellDiameter = LvArray::math::sqrt< real64 >( cellDiameter );
real64 const invCellDiameter = 1.0/cellDiameter;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest that in the next PR we move this piece of code computing the cell diameter could be moved to ComputationalGeometry since it seems generic and could be useful for others.

Comment on lines +115 to +116
array1d< real64 > monomInternalIntegrals( 3 );
LvArray::tensorOps::fill< 3 >( monomInternalIntegrals, 0.0 );
Copy link
Contributor

Choose a reason for hiding this comment

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

These two lines could be simplified to:

real64 monomInternalIntegrals[ 3 ] = { 0.0 };

Comment on lines +7 to +10
void ConformingVirtualElementOrder1::ComputeProjectors( MeshLevel const & mesh,
localIndex const & regionIndex,
localIndex const & subRegionIndex,
localIndex const & cellIndex )
Copy link
Contributor

Choose a reason for hiding this comment

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

In the next PR when you want to call this function inside a kernel to compute the basis functions and the stabilization matrix in each cell, you may have to update the signature of this function a little bit.

Instead of passing the mesh and the triplet of indices that identifies a cell, I think you would directly pass the volume and center of the particular cell, plus some arrayViews (say, faceCenters, faceNormals, edgeToNodes, etc), arraySlices (say, elemToFaces and cellToNodes for the particular cell) that you need inside the function. You would also pass a reference to the three arrays (basis functions, derivatives, and stabilization matrix) that you want to populate in this function. With these changes (and maybe some small others) it will be possible to call ComputeProjectors inside a kernel.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree. We will make use of a StackVariables struct, which is set up in a kernel launch looping over cells of a subregion before entering the quadrature point kernel (see kernelLaunch in the base interface for finite element kernels). For example, in SolidMechanicsSmallStrainQuasiStaticKernel.hpp:

  • stack arrays for the vertex coordinates, displacement, ... are added to StackVariables (see)
  • values are copied in the setup method (see)

We need to carefully evaluate what we should copy and what we should recompute.

array1d< real64 > basisBoundaryIntegrals( numCellPoints );
array2d< real64 > basisTimesNormalBoundaryInt( 3, numCellPoints );
array2d< real64 > basisTimesMonomNormalDerBoundaryInt( numCellPoints, 3 );
array1d< real64 > monomBoundaryIntegrals( 4 );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
array1d< real64 > monomBoundaryIntegrals( 4 );
real64 monomBoundaryIntegrals[ 4 ] = { 0.0 };

and in general I think you can replace (maybe in the next PR) all the array1d and array2d that have a fixed size with C-arrays.

Copy link
Contributor

@castelletto1 castelletto1 left a comment

Choose a reason for hiding this comment

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

Thanks @andrea-borio! The PDF with the pseudo-code of the implementation was extremely useful. Let's try to have this PR merged as soon as possible and start the next one right after.

If the usual unit test keeps failing for Pangea2, we might have to deactivate them again (after consulting with @TotoGaz).

array1d< real64 > & threeDMonomialIntegrals );

public:
ConformingVirtualElementOrder1() {}
Copy link
Contributor

@castelletto1 castelletto1 Jan 15, 2021

Choose a reason for hiding this comment

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

Suggested change
ConformingVirtualElementOrder1() {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My compiler says constructors cannot be declared ‘virtual’ [-fpermissive]

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, remove the constructor all together.

Comment on lines +7 to +10
void ConformingVirtualElementOrder1::ComputeProjectors( MeshLevel const & mesh,
localIndex const & regionIndex,
localIndex const & subRegionIndex,
localIndex const & cellIndex )
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree. We will make use of a StackVariables struct, which is set up in a kernel launch looping over cells of a subregion before entering the quadrature point kernel (see kernelLaunch in the base interface for finite element kernels). For example, in SolidMechanicsSmallStrainQuasiStaticKernel.hpp:

  • stack arrays for the vertex coordinates, displacement, ... are added to StackVariables (see)
  • values are copied in the setup method (see)

We need to carefully evaluate what we should copy and what we should recompute.

@TotoGaz
Copy link
Contributor

TotoGaz commented Jan 16, 2021

If the usual unit test keeps failing for Pangea2, we might have to deactivate them again (after consulting with @TotoGaz).

Yes, deactivate what you need. Just open an issue so we keep track of it.

@andrea-borio andrea-borio added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Jan 19, 2021
@andrea-borio
Copy link
Contributor Author

andrea-borio commented Jan 19, 2021

I labeled the PR as ready to be merged even though there are some issues with Travis. Let's discuss this at today's meeting.

@castelletto1 castelletto1 merged commit 99613c1 into develop Jan 19, 2021
@castelletto1 castelletto1 deleted the feature/borio/vem branch January 19, 2021 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run CUDA builds Allows to triggers (costly) CUDA jobs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants