Feature/toolboxes refactoring #1416

vincentchabannes · 2020-01-23T19:39:11Z

Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes?
Have you successfully run the Feel++ testsuite with your changes locally?
Have you written Doxygen comments in your contribution ?

Update in feelpp-lib :

operatorinterpolation : Support interpolation operator with expression #1417
feelvf : Merge GinacExVf and GinacMatrix #1475
feelvf : symbolic expression can depend on an expression which can depend on another expresssions etc..
feelvf : support differentiation of expression with respect to a symbol name
feelvf : refactoring of mat/vec keywords, replace boost::fusion by boost::hana.
feelmodels : refactoring of ModelParameters (not only constant parameter)
feells : TO COMPLETE

Major update in feelpp-toolboxes :

core : new definition of toolbox thanks to ModelFields and ModelPhysics class. Factorize and simplify the definition of symbol and the genration of materials properties
post-processing update :
-- export math expressions from JSON
-- factorize code of post-processing in toolboxes
material properties:
-- unique material properties share by mono/multiphysics toolboxes
-- generate automatically all symbol expressions
-- create a keyword that allows defining a material property for all materials (the expression is selected from the context, .i.e. the mesh element )
heat toolbox :
-- new post-process measures : TO COMPLETE
fluid toolbox :
-- no use a composite space for fluid/pressure
-- implement rigid particle bc
levelset/multifluid :
-- TO COMPLETE
coefficient form pdes (new toolbox) :
-- a pde or a system of pdes can be solved by given the coefficients: m d^2 u/dt^2 + d du/dt + \nabla \cdot ( -c \nabla u ) + \beta cdot \nabla u + a u = f where m, d, c, \beta, a, f are the coefficients.
-- the type of finite element used in each equation can be differents

vincentchabannes · 2020-02-18T20:14:23Z

@metivett test_levelset.cpp doesn't compile. Could you fix asap?

metivett · 2020-02-19T13:56:36Z

test_levelset.cpp doesn't compile. Could you fix asap?

@vincentchabannes Done ! I also merged some fixes in the MultiFluid toolbox from the feature/ls branch.
I am working on updating all the testcases (only multifluid_square is done at the moment).

vincentchabannes · 2020-03-04T11:20:48Z

@metivett what is the goal of syncDofs (feelpp/feel/feeldiscr/syncdofs.hpp).
We have already the sync keyword which seems to do the same thing I think, no? see #671 . And I have added also the close args with on(..) projection which can apply easily the sync.

metivett · 2020-03-04T13:52:27Z

@metivett what is the goal of syncDofs (feelpp/feel/feeldiscr/syncdofs.hpp).
We have already the sync keyword which seems to do the same thing I think, no? see #671 . And I have added also the close args with on(..) projection which can apply easily the sync.

@vincentchabannes Actually the goal was to allow a simpler interface (using only a lambda function instead of a functor derived from SyncOp), and to enable the synchronisation of a restricted set of dofs specified by an element range (and not a set of dofs). We could extend the sync function to provide these functionalities for sure, but I did not want to break other codes...

vincentchabannes · 2020-03-04T13:57:49Z

I do not agree, the interface allows to support range, as you can see here.
And the options with ComponentType::NO_COMPONENT and the boolean are optionally(optimization)

And it supports range of elements, faces, edges, points.

It's hard to maintain another mpi code.

metivett · 2020-03-04T14:05:50Z

Ok, but the functor approach is a bit cumbersome I think, and the syncDofs function is more generic than the sync one since it allows to synchronise any template vector (the vector and datamap types are separate parameters, see eg here ).

vincentchabannes · 2020-03-04T14:23:56Z

There is some optimisation for the classic case ( =) and I treat only the dofs on interprocess, this is not the case with your implementation (some loop over all range).
we can add easily an interface for your case.

metivett · 2020-03-04T14:39:36Z

I don't really see what you can optimise further, at some point you need to find the interprocess dofs which are in your range (in my case it is done in the function while you do it separately with the functionspace->dofs() call). But if you want we can merge the two functions ;-) !

vincentchabannes · 2020-03-04T14:47:47Z

I doesn't call localToGlobal and this has a cost (you do this for all element)
The dofs at interprocess are identified only one time, we can reuse it after for each sync
In the case of = operator, we don't need to apply 2 passes of mpi communication, but only one.

metivett · 2020-03-04T15:01:47Z

I don't really agree, I just think the purposes are just a bit different :
1 -> if you don't call localToGlobal, then you need to send all the ghost interprocess dofs, not just the one which are in your range, which can lead to unneeded communications
2 -> the dofs which are both interprocess and in your range can't be reused (the range changes)
3 -> ok, but I had a generic lambda synchronisation in mind

But I will not fight to death for my function ;-), as I said we can refactor the sync one to enable my use (and maybe the flexibility of lambdas).

vincentchabannes · 2020-03-04T15:11:31Z

For finish :)

No, the informations are in DataMap. but you need to communicate if the range is not contains the active dofs. Try with a random partitioning
Maybe in your case, but generally no.

I comment on this function because I need to review all mpi comm, there are some troubles with last boost (>=1.70) #1296 #1438.

…lpp into feature/toolboxes-refactoring

…cess dofIds to prevent calls to searchGlobalProcessDof which is costly for ghost dofs)

…elpp/feelpp into feature/toolboxes-refactoring

…lpp into feature/toolboxes-refactoring

…on and conservativeFluxSource

@lberti

…upling with gravity if enable (@lberti)

- can pass a mesh support instead of a mesh : allow to force some elements extracted (that are isolated in mesh support) to be ghost (can fix some special case of partitioning) - up mpi comm for #1296

…f partial mesh support

prudhomm

I lost all the comments I made.
about 40 of them. I approve the PR

prudhomm · 2020-06-13T21:19:35Z

feelpp/feel/feelalg/backend.hpp

@@ -362,6 +362,12 @@ class FEELPP_EXPORT Backend : public BackendBase, public std::enable_shared_from

    virtual sparse_matrix_ptrtype newZeroMatrix( datamap_ptrtype const& dm1, datamap_ptrtype const& dm2 ) = 0;

+    virtual sparse_matrix_ptrtype newIdentityMatrix( datamap_ptrtype const& dm1, datamap_ptrtype const& dm2 )


why not a pure virtual like above ?

prudhomm · 2020-06-13T21:24:59Z

feelpp/feel/feelalg/backendpetsc.hpp

@@ -294,6 +294,18 @@ class FEELPP_EXPORT BackendPetsc : public Backend<T,SizeT>
        return mat;
    }

+    sparse_matrix_ptrtype newIdentityMatrix( datamap_ptrtype const& domainmap, datamap_ptrtype const& imagemap )
+    {
+        graph_ptrtype sparsity_graph( new graph_type( imagemap,imagemap ) );


use std::make_shared

it is slightly less expensive, only one heap allocation

prudhomm · 2020-06-13T21:31:29Z

feelpp/feel/feelcore/tuple_utils.hpp

+            static_cast<F&&>(f), static_cast<ItT1&&>(itBegin), static_cast<ItT2&&>(itEnd)
+            );
+}
+


@metivett could you add some basic documentation ala doxygen ?

prudhomm · 2020-06-13T21:31:46Z

feelpp/feel/feelcore/tuple_utils.hpp

+
+} // namespace detail
+
+template <typename F, typename ItT1, typename ItT2>


@metivett could you add some basic documentation ala doxygen ?

prudhomm · 2020-06-13T21:36:52Z

feelpp/feel/feeldiscr/operatorinterpolation.hpp

+                    break;
+                }
+            }
+            CHECK( find ) << "not find a compatible dof\n ";


better wording "a compatible dof was not found between image and domain dof" ?

prudhomm · 2020-06-17T20:08:48Z

feelpp/feel/feelvf/detail/ginacexvf.hpp

                            {
-                                auto const& theexpr = std::get<1>( e );


would it be possible to wrap the tuple into a class or struct with a more explicit interface ?
if it stays private and is not exposed, this is not necessary

prudhomm · 2020-06-17T20:09:46Z

feelpp/feel/feelvf/detail/ginacexvf.hpp

+    bool hasAtLeastOneSymbolDependency( AnElementOfSymbolExprType const& e ) const
+    {
+        bool res = false;
+        if ( std::get<2>( e ).empty() )


same as above

prudhomm · 2020-06-17T20:13:36Z

feelpp/feel/feelvf/expr.hpp

@@ -877,11 +1018,18 @@ public :

 template <typename ExprT>
 Expr<ExprT>
-expr( ExprT const& exprt )
+expr( ExprT const& exprt, typename std::enable_if_t<is_vf_expr_v<ExprT> >* /*= nullptr*/ )


why no default value ?
you could also have this in the template rather than the argument of the function. that would (de)select earlier in the compilation process the function

prudhomm · 2020-06-17T20:13:43Z

feelpp/feel/feelvf/expr.hpp

 {
    return Expr<ExprT>( exprt );
 }

+template <typename ExprT>
+Expr<ExprT>
+expr( ExprT && exprt, typename std::enable_if_t<is_vf_expr_v<ExprT> >* /*= nullptr*/ )


prudhomm · 2020-06-17T20:16:45Z

feelpp/feel/feelvf/geometricdata.hpp

+        }                                                               \
+                                                                        \
+        template <typename TheSymbolExprType>                           \
+            bool hasSymbolDependency( std::string const& symb, TheSymbolExprType const& se ) const \


how about testing nx, ny nz symbols ?

or is it not relevant ?

vincentchabannes added type:refactoring 🧰 component: toolboxes labels Jan 23, 2020

vincentchabannes self-assigned this Jan 23, 2020

vincentchabannes and others added 16 commits March 11, 2020 16:57

export translational and angular velocity in csv measure file @lberti

e2fe0fa

Merge branch 'feature/toolboxes-refactoring' of github.com:feelpp/fee…

f2ebc5e

…lpp into feature/toolboxes-refactoring

fix compilation solid toolbox

24e422b

call updateParameterValue before solve

94e0156

split sources compilation of heat/electric/thermoelectric

d0c4ddf

change definition of model field tag

b4734a9

Merge branch 'feature/remesh' into feature/toolboxes-refactoring

7f5f58b

refactoring of modelfieds (work in progress)

58b55db

fix compilation fluid/solid/heatfluid + split heatfluid

fcef1b4

fix compilation (but not yet working)

c260f56

fix compilation

f48c148

minor up

782213a

start adr refactoring

0f827c8

Remove unneeded sync and marchNarrowBand in fastmarching

2db9f32

Improve fastmarching communications (cache globalCluster to globalPro…

4e30fa3

…cess dofIds to prevent calls to searchGlobalProcessDof which is costly for ghost dofs)

Merge branch 'feature/toolboxes-refactoring' of https://github.com/fe…

7a1f3d8

…elpp/feelpp into feature/toolboxes-refactoring

vincentchabannes and others added 28 commits June 18, 2020 15:23

defined Expr::hasSymbolDependencyOnCoordinatesInSpace<Dim>

8b0bdf2

fix eval of div( A grad(u) ) if A (scalar case only) depends on x,y,z

84af282

up coefficientformpdes : automatic solver selection

608a691

fix compilation

3eb093f

fix compilation

2bf83db

fix compilation

745b8be

up SymbolExpr : replace tuple by a struct @metivett @prudhomm

f583668

Merge branch 'feature/toolboxes-refactoring' of https://github.com/fe…

c43ea95

…elpp/feelpp into feature/toolboxes-refactoring

Up cfg files fieldsplit crashing #1484 @vincentchabannes

6caff55

up coefficientformpdes : implement theta time step

b0abede

Merge branch 'feature/toolboxes-refactoring' of github.com:feelpp/fee…

0d1a68e

…lpp into feature/toolboxes-refactoring

fix #1484 @lberti @prudhomm

924c1d4

up coefficientformpdes by adding new coeff : conservativeFluxConvecti…

81ef177

…on and conservativeFluxSource

add predator-prey with coefficientformpdes toolbox

e0fbac4

fix solver PtAP with PCD and PMM preconditioners @lberti

6e0d79a

up fluid toolbox : add gravity force config in json + support body co…

525b63f

…upling with gravity if enable (@lberti)

add fluid example with body and gravity (@lberti)

53b5b04

fix interpolate with partial mesh support

9c2e620

remove my test in cfg [ci skip]

92d85dd

add accessors

cdf8da7

continue fix #1296 (not use vector directly in comm)

01bfdb4

up createSubmesh :

02d1bce

- can pass a mesh support instead of a mesh : allow to force some elements extracted (that are isolated in mesh support) to be ghost (can fix some special case of partitioning) - up mpi comm for #1296

use velocity meshsupport in createsubmesh (rigid body bc submesh)

6ae8cf4

quick fix of interpolation between fluid and meshale spaces in case o…

82d877e

…f partial mesh support

up parameter values with gravity and body

63fbed9

minor up

98c34be

remove cfg

3c25b14

not use std vector in mpi comm #1296

8694fcc

prudhomm approved these changes Jul 3, 2020

View reviewed changes

prudhomm merged commit 27aa80d into develop Jul 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/toolboxes refactoring #1416

Feature/toolboxes refactoring #1416

vincentchabannes commented Jan 23, 2020 •

edited

vincentchabannes commented Feb 18, 2020

metivett commented Feb 19, 2020

vincentchabannes commented Mar 4, 2020 •

edited

metivett commented Mar 4, 2020 •

edited

vincentchabannes commented Mar 4, 2020 •

edited

metivett commented Mar 4, 2020

vincentchabannes commented Mar 4, 2020

metivett commented Mar 4, 2020

vincentchabannes commented Mar 4, 2020 •

edited

metivett commented Mar 4, 2020

vincentchabannes commented Mar 4, 2020

prudhomm left a comment

prudhomm Jun 13, 2020

prudhomm Jun 13, 2020

prudhomm Jun 13, 2020

prudhomm Jun 13, 2020

prudhomm Jun 13, 2020

prudhomm Jun 13, 2020

prudhomm Jun 17, 2020

prudhomm Jun 17, 2020

prudhomm Jun 17, 2020

prudhomm Jun 17, 2020

prudhomm Jun 17, 2020

prudhomm Jun 17, 2020

		@@ -362,6 +362,12 @@ class FEELPP_EXPORT Backend : public BackendBase, public std::enable_shared_from

		virtual sparse_matrix_ptrtype newZeroMatrix( datamap_ptrtype const& dm1, datamap_ptrtype const& dm2 ) = 0;

		virtual sparse_matrix_ptrtype newIdentityMatrix( datamap_ptrtype const& dm1, datamap_ptrtype const& dm2 )


		} // namespace detail

		template <typename F, typename ItT1, typename ItT2>

Feature/toolboxes refactoring #1416

Feature/toolboxes refactoring #1416

Conversation

vincentchabannes commented Jan 23, 2020 • edited

vincentchabannes commented Feb 18, 2020

metivett commented Feb 19, 2020

vincentchabannes commented Mar 4, 2020 • edited

metivett commented Mar 4, 2020 • edited

vincentchabannes commented Mar 4, 2020 • edited

metivett commented Mar 4, 2020

vincentchabannes commented Mar 4, 2020

metivett commented Mar 4, 2020

vincentchabannes commented Mar 4, 2020 • edited

metivett commented Mar 4, 2020

vincentchabannes commented Mar 4, 2020

prudhomm 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincentchabannes commented Jan 23, 2020 •

edited

vincentchabannes commented Mar 4, 2020 •

edited

metivett commented Mar 4, 2020 •

edited

vincentchabannes commented Mar 4, 2020 •

edited

vincentchabannes commented Mar 4, 2020 •

edited