Why use cond properties and not k in HDG toolbox ?

[prudhomm]

in the HDG toolbox, the property "cond" is defined instead of "k" which would be the standard one for the conductivity. Why is that?

The main issue is that it is not defined in the material properties data structures as it would be for the other toolboxes. It is only available in the ptree. It seems quite inconsistent and error-prone.

Moreover, for the python wrapping, it is a pain as it would require specific handling of this material property. I spent quite some time on this issue

[lsala]

I think that at the beginning, the first implementation of HDG Poisson by Romain and Daniele was for thermal applications and all the drafts for theory were written for that.
Then to keep theory drafts matched with code, we never modified it

[romainhild]

In the case of thermoelectric problem, we have two Poisson toolboxes for two different physics with different conductivities. If we use the data structure of modelmaterial we need to have two different json file.
The cond key word can be change using the conductivity_json option, which default to cond but could be change to k.
By data structure, you mean use material->propertyConstant("k") ? Since there is no function material->k().

What could be done is here

feelpp/feelpp/feel/feelmodels/modelmaterials.cpp

Lines 76 to 81 in 7ad447d

	std::set<std::string> matProperties = { "rho","mu","Cp","Cv","Tref","beta",
	"k","k11","k12","k13","k22","k23","k33",
	"E","nu","sigma","C","Cs","Cl","L",
	"Ks","Kl","Tsol","Tliq" };
	for ( std::string const& prop : matProperties )
	this->setProperty( prop,M_p );

instead of this, we use setProperty for every child found different than physics, markers and filename.
And in the toolbox, instead of using material.getScalar(M_conductivityKey); we would use material.propertyExprScalar(M_conductivityKey).
What do you think ?

[prudhomm]

what is the problem with having two json files? the two problems have their own BC and materials associated, no ?

Moreover on ModelMaterial there is a mix of ptreeand map usage which is confusing. In my opinion, the ptreeshould only be used to configure the material data structure either at constructor or through an API. Afterwards the user accesses only the M_materialProperties data structure through the interface

[prudhomm]

This issue is strongly related to #1287 which is needed for OpenTURNS and #1259 at least.
we have to be able to update the properties from Python and re-run the toolbox as many times we want. Unfortunately cond breaks the interface and cannot be updated as the other properties

[prudhomm]

We could indeed add the possibility to register automatically new properties

[romainhild]

I agree that we should only use M_materialProperties, it is just that it didn't exist when I wrote the toolbox. The use of ModelExpression is of course cleaner.
If we register automatically new properties, I don't see the need for the ptree. And the changes would be minimal.

[romainhild]

For the two, or more, json files, I think it is both a waste of time and a source of errors.

[romainhild]

So this would work for you with python ?

instead of this, we use setProperty for every child found different than physics, markers and filename.
And in the toolbox, instead of using material.getScalar(M_conductivityKey); we would use material.propertyExprScalar(M_conductivityKey).

[prudhomm]

I would prefer to use standard name properties defined by physics.

regarding the two json, what about this https://github.com/feelpp/feelpp/tree/develop/toolboxes/hdg/HL-31 ?
I am confused

[metivett]

what is the problem with having two json files? the two problems have their own BC and materials associated, no ?

Moreover on ModelMaterial there is a mix of ptreeand map usage which is confusing. In my opinion, the ptreeshould only be used to configure the material data structure either at constructor or through an API. Afterwards the user accesses only the M_materialProperties data structure through the interface

@prudhomm Regarding this mix between the inner ptree and the map interface, I think it brings confusion indeed ; but the map approach is too restrictive, and quite difficult to extend. For example, I needed a new BoundaryCondition type for the particle injector, and had to use directly the info from the ptree.
I think we should maybe only use one representation for Properties, based on the ptree approach, which is really flexible. Then we could add "markers and the like" access functions, but only used as syntactic sugar (similar to the different search algorithms that can be used on a tree, e.g. the markers function would be similar to a breadth-first search).

[prudhomm]

the ptree data structure is not rich enough. there should be a mapping between the ptree and the associated data structures which provide meaning, analysis and present the data to the toolbox . If the data structures are not rich enough(there are definitely things that can be improved), then we discuss it.
I really got bitten by the interface and the mixing ptree/map; it is currently messy in ModelMaterial(s)

[romainhild]

Regarding this I would suggest to pass every function that uses the ptree directly in deprecated, and see if we need something else in ModelMaterial.
Maybe it could be useful also to store in ModelExpression the string of the expression, so that if someone wants to do something not standard with material properties he is able to retrieve the expression and do whatever he wants.

[prudhomm]

@vincentchabannes @romainhild I am closing the issue. If I understand correctly, this is taken care of by the Physics section and the vocabulary we can put on top of a generic toolbox with the expression system. is that correct ?

[romainhild]

It is correct, with the Physics you can use different terms for the primal and dual variables, but it should be improved to support conductivities and boundary conditions.

feelpp/toolboxes/feel/feelmodels/hdg/enums.cpp

Lines 9 to 12 in 44cb184

	std::map<MixedPoissonPhysics,std::map<std::string,std::string> > MixedPoissonPhysicsMap = {
	{ MixedPoissonPhysics::None, {{"potentialK", "potential"},{"fluxK","flux"},{"keyword","poisson"},{"potentialSymbol","P"},{"fluxSymbol","F"}} },
	{ MixedPoissonPhysics::Electric, {{"potentialK", "electric-potential"},{"fluxK","current-density"},{"keyword","electric"},{"potentialSymbol","P"},{"fluxSymbol","C"}} },
	{ MixedPoissonPhysics::Heat, {{"potentialK", "temperature"},{"fluxK","heat-flux"},{"keyword","heat"},{"potentialSymbol","T"},{"fluxSymbol","F"}} }