Feature/borio/vem refactoring #1288

andrea-borio · 2021-01-24T13:45:22Z

Related to https://github.com/GEOSX/integratedTests/pull/129

These methods could be moved to ComputationalGeometry.hpp.

…nd removing the use of tensorOps.

Removed virtual method in base class (to be reinstated). Updated unit tests according to new method signature.

This also serves as example of use of the method ComputeProjectors.

Update documentation. Update unit tests accordingly.

Also added GEOSX_HOST_DEVICE macro to all member methods.

…efactoring

Add empty loop on sub-regions and getters.

Fix bug in xml files.

…efactoring

andrea-borio · 2021-02-23T13:09:51Z

Since the LaplaceVEM class is not yet intended to use kernel launches, I commented all GEOSX_HOST_DEVICE flags in VEM classes, that generated compilation errors with cuda. The errors were due to the fact that the method ConformingVirtualElementOrder1::ComputeProjectors cannot access data members from the device.

francoishamon

Looks good to me, thank you @andrea-borio

francoishamon · 2021-03-08T19:56:38Z

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.hpp

+// are implemented at the SolverBase class level and can thus be used in Laplace without needing reimplementation.
+
+//START_SPHINX_INCLUDE_02
+class LaplaceVEM : public SolverBase


One small change that could potentially reduce significantly the amount of code in the new LaplaceVEM would be to either A) inherit from LaplaceFEM or B) create a base class containing all the common functions and inherit from it.

In the LaplaceVEM class, you would only need to 1) define the catalog name, 2) define SetupSystem (to call SolverBase::SetupSystem, and 3) define AssembleSystem.

If the plan for the SimplePDEs is to ultimately have a LaplaceDG, LaplaceMFD, etc, maybe implementing B is the way to go, but I don't know what others think.

I agree with option B, something like a LaplaceBase class would be the way to go. I can try to do that if you want.

Yes, let's do a LaplaceBase class, but in a separate PR now that the overlap is clear.

francoishamon · 2021-03-08T19:59:32Z

src/coreComponents/virtualElement/CMakeLists.txt

  VirtualElementBase.hpp
  )
 #
 # Specify all sources
 #
 set( virtualElement_sources
-  ConformingVirtualElementOrder1.cpp
+  dummy.cpp


If you don't have a dummy.cpp in this folder, then the content of the folder is not compiled? Is there a way to make this work without the dummy.cpp?

I was able to make things work without dummy.cpp mimicking what was done in src/coreComponents/math/CMakeLists.txt.

src/coreComponents/virtualElement/ConformingVirtualElementOrder1.hpp

src/coreComponents/virtualElement/VirtualElementBase.hpp

src/coreComponents/virtualElement/ConformingVirtualElementOrder1_impl.hpp

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp

…ture points.

…efactoring

joshua-white · 2021-03-23T16:17:47Z

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp

+ * Where can I find an example of what it does?
+ * --------------------------------------------
+ *
+ * Integrated tests associated to this solver are found in the ./integratedTests/ folder


@andrea-borio, Does this actually describe your integrated tests? This header looks like cut and paste from the FEM version, so we should make sure it is still accurate.

It is indeed a cut and paste of the text that you wrote in LaplaceFEM.cpp, but I changed all mentions to finite elements into mentions to virtual elements. The text is valid, it tells the truth. When we write a base class, we will have just one.

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp

joshua-white · 2021-03-23T16:27:17Z

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.hpp

+  {
+    SteadyState,
+    ImplicitTransient,
+    ExplicitTransient


I don't know why we have the explicit option for Laplace at all. Perhaps something to remove when we put the base class in place.

joshua-white · 2021-03-23T16:28:42Z

src/coreComponents/physicsSolvers/simplePDE/integratedTests/10x10x10Hex_LaplaceVEM.xml

+  <Solvers>
+    <LaplaceVEM
+        name="laplace"
+        discretization="FE1"


joshua-white

This looks great. I pointed out a few things we should clean up, but probably in a follow up PR. Many will be addressed by creating a LaplaceBase. We will also have to figure out what to do with the discretization key.

rrsettgast

Mostly some formatting changes. You can look at the suggestions in the "Files changed" tab and batch them into a single commit, or you can make them manually yourself. The LEAF comment should be addressed.

rrsettgast · 2021-03-23T17:13:31Z

src/coreComponents/virtualElement/CMakeLists.txt

-  set( dependencyList common)
-endif()
+# if( BUILD_OBJ_LIBS)
+#   set( dependencyList dataRepository codingUtilities)


You have to be dependent on something don't you? At the very least you must be dependent on common?

These dependencies are commented out because I am currently not compiling anything in the VEM folder and cmake fails if I declare dependencies. I left these lines as comments for future reference.

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp

rrsettgast · 2021-03-23T17:23:10Z

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp

+  arrayView1d< globalIndex const > const & dofIndex =
+    nodeManager.getReference< array1d< globalIndex > >( dofManager.getKey( m_fieldName ) );
+
+  real64 const diffusion = 1.0;


maybe make this an input variable?

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp

rrsettgast · 2021-03-23T17:25:28Z

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp

+void LaplaceVEM::resetStateToBeginningOfStep( DomainPartition & GEOSX_UNUSED_PARAM( domain ) )
+{}


Does this not need to be filled?

src/coreComponents/virtualElement/unitTests/testVirtualElementBase.cpp

…efactoring

andrea-borio added 22 commits January 22, 2021 12:22

Use C-arrays in ConformingVirtualElementOrder1::ComputeFaceIntegrals

960d784

Add methods to compute diameter in ConformingVirtualElementOrder1.

fe6d140

These methods could be moved to ComputationalGeometry.hpp.

Using C-arrays in ConformingVirtualElementOrder1::ComputeProjectors a…

5c8f07b

…nd removing the use of tensorOps.

Change inputs of ConformingVirtualElementOrder1::ComputeFaceIntegrals

132de9a

Change inputs of ConformingVirtualElementOrder1::ComputeProjectors.

f67bda3

Removed virtual method in base class (to be reinstated). Updated unit tests according to new method signature.

Remove unused parameters from method ComputeProjectors().

fa0580b

Implement separate function to test all cells of a MeshLevel.

1bb64f7

This also serves as example of use of the method ComputeProjectors.

Add minimal getters to VEM classes.

3e27d37

Update documentation. Update unit tests accordingly.

Removed methods and classes preventing compilation on Lassen.

ceada88

Templatize and make static class ConformingVirtualElementOrder1.

eb9490a

Also added GEOSX_HOST_DEVICE macro to all member methods.

testVirtualElementBase using static methods.

469782e

Using fixed size C-arrays for projections in VEM classes.

0417e13

Implement methods in base class.

de4f203

Remove unused template parameter MAXCELLFACES.

b5fb666

Merge remote-tracking branch 'origin/develop' into feature/borio/vemR…

25440df

…efactoring

Add LaplaceVEM class as a copy of LaplaceFEM.

9254f22

Commented method LaplaceVEM::setupSystem

75406ee

WIP on LaplaceVEM::assembleSystem.

7e11d1d

Add empty loop on sub-regions and getters.

Complete (non-CUDA) implementation of LaplaceVEM::assembleSystem.

8140141

Add integrated tests for LaplaceVEM solver.

669d8ff

Set number of quadrature points for VEM to 1 (weight is cellVolume).

46e0031

Fix bug in xml files.

Merge remote-tracking branch 'origin/develop' into feature/borio/vemR…

3571310

…efactoring

andrea-borio marked this pull request as ready for review February 21, 2021 17:37

andrea-borio requested review from castelletto1, francoishamon, joshua-white and rrsettgast February 21, 2021 17:38

andrea-borio added 3 commits February 22, 2021 09:51

Correct positioning of GEOSX_HOST_DEVICE

d5592fd

Add GEOSX_HOST_DEVICE to ConformingVirtualElementOrder1 members.

39f2f5a

Add .rst files

4470525

andrea-borio added 4 commits February 22, 2021 18:32

Removed GEOSX_HOST_DEVICE from ComputeDiameter methods

f7baeb8

Remove GEOSX_HOST_DEVICE from VirtualElementBase

22c0394

Specify scope of method ComputeDiameter

7a6473b

Try removing specifications of template parameters of ComputeDiameter.

8062d3f

andrea-borio added the flag: ready for review label Feb 23, 2021

francoishamon approved these changes Mar 8, 2021

View reviewed changes

andrea-borio added 7 commits March 10, 2021 18:52

Add missing const.

6be8cbc

Add missing const

670a58c

Changed region loop in LaplaceVEM::assembleSystem

b0e2182

Use camelCase on all function names.

db783c8

Implemented getters for number of support points and number of quadra…

f2c0698

…ture points.

Removing dummy.cpp and fixing compilation issues.

05313a5

Merge remote-tracking branch 'origin/develop' into feature/borio/vemR…

211cb05

…efactoring

joshua-white reviewed Mar 23, 2021

View reviewed changes

src/coreComponents/physicsSolvers/simplePDE/LaplaceVEM.cpp Outdated Show resolved Hide resolved

joshua-white reviewed Mar 23, 2021

View reviewed changes

joshua-white approved these changes Mar 23, 2021

View reviewed changes

joshua-white added ci: run CUDA builds Allows to triggers (costly) CUDA jobs and removed flag: ready for review labels Mar 23, 2021

rrsettgast requested changes Mar 23, 2021

View reviewed changes

andrea-borio and others added 5 commits March 26, 2021 11:16

Remove commented code.

a908535

Changes to code formatting.

3dca239

Merge remote-tracking branch 'origin/develop' into feature/borio/vemR…

8f661c7

…efactoring

Merge branch 'develop' into feature/borio/vemRefactoring

5ce7855

updated integratedTests submodule

583cea4

rrsettgast approved these changes Apr 1, 2021

View reviewed changes

francoishamon merged commit 833e470 into develop Apr 1, 2021

francoishamon deleted the feature/borio/vemRefactoring branch April 1, 2021 18:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/borio/vem refactoring #1288

Feature/borio/vem refactoring #1288

andrea-borio commented Jan 24, 2021 •

edited by francoishamon

Loading

andrea-borio commented Feb 23, 2021

francoishamon left a comment

francoishamon Mar 8, 2021

andrea-borio Mar 12, 2021

joshua-white Mar 23, 2021

francoishamon Mar 8, 2021

andrea-borio Mar 11, 2021

joshua-white Mar 23, 2021

andrea-borio Mar 26, 2021

joshua-white Mar 23, 2021

joshua-white Mar 23, 2021

joshua-white left a comment

rrsettgast left a comment

rrsettgast Mar 23, 2021

andrea-borio Mar 26, 2021

rrsettgast Mar 23, 2021

rrsettgast Mar 23, 2021

		void LaplaceVEM::resetStateToBeginningOfStep( DomainPartition & GEOSX_UNUSED_PARAM( domain ) )
		{}

Feature/borio/vem refactoring #1288

Feature/borio/vem refactoring #1288

Conversation

andrea-borio commented Jan 24, 2021 • edited by francoishamon Loading

andrea-borio commented Feb 23, 2021

francoishamon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshua-white left a comment

Choose a reason for hiding this comment

rrsettgast left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrea-borio commented Jan 24, 2021 •

edited by francoishamon

Loading