[Core][ConstitutiveLaw]Inconsistency with CL::Flags

AlejandroCornejo commented 6 years ago

Good Afternoon,

Me and @loumalouomega have been thinking to add a new CL::Flag named "IS_SLAVE" to be used when a certain CL is controled by another CL, Is it possible? The mentioned inconsistency can be seen in the following lines of the constitutive_law.cpp:

KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, USE_ELEMENT_PROVIDED_STRAIN,  0 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, COMPUTE_STRESS,               1 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, COMPUTE_CONSTITUTIVE_TENSOR,  2 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, COMPUTE_STRAIN_ENERGY,        3 );

KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, ISOCHORIC_TENSOR_ONLY,        4 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, VOLUMETRIC_TENSOR_ONLY,       5 );

KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, MECHANICAL_RESPONSE_ONLY,     6 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, THERMAL_RESPONSE_ONLY,        7 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, INCREMENTAL_STRAIN_MEASURE,   8 );

KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, INITIALIZE_MATERIAL_RESPONSE, 9 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, FINALIZE_MATERIAL_RESPONSE,  10 );

KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, FINITE_STRAINS,              1 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, INFINITESIMAL_STRAINS,       2 );

KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, THREE_DIMENSIONAL_LAW,       3 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, PLANE_STRAIN_LAW,            4 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, PLANE_STRESS_LAW,            5 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, AXISYMMETRIC_LAW,            6 );

KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, U_P_LAW,                     7 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, ISOTROPIC,                   8 );
KRATOS_CREATE_LOCAL_FLAG( ConstitutiveLaw, ANISOTROPIC,                 9 );

We wonder why the numbering is repeated for some flags?

Thank you

@RiccardoRossi @josep-m-carbonell

loumalouomega commented 6 years ago

I prefer just MASTER and SLAVE instead of IS_SLAVE

josep-m-carbonell commented 6 years ago

There is no inconsistency. They are different type of flags, for different purposes, and they must not be mixed. The first ones define calculation Options and the second ones Law types and features. If you need SLAVE and MASTER flags for the CLWS, maybe there is a bad design of the constitutive laws. However, I can say nothing because I don't know when and why you need them.

loumalouomega commented 6 years ago

We need it to define combinations of CL, where one law will be the master and the other the slave

loumalouomega commented 6 years ago

And the current flags can be for different purposes, but the all of them are local flags for the CL, so we are concerned that the ID could be a problematic

josep-m-carbonell commented 6 years ago

Take into account that the number of flags is limited.

loumalouomega commented 6 years ago

32 is the limit, we are still far from the limit, and we are asking if does look like a good idea

josep-m-carbonell commented 6 years ago

Again, you don't have to mix these flags. I don't know if it is a good idea to Add MASTER and SLAVE because I don't know how you want to use them. Maybe you can explain your design a little bit more in order to be able to have an opinion about.

AlejandroCornejo commented 6 years ago

okay let me explain the idea a bit:

The problem is just to create a Visco-Plasticity Law by combining the already working visco and plasticity laws... We would use that flag to inform the plasticity that some operations can be different if it is slave or not for example.

loumalouomega commented 6 years ago

Myabe the name can be improved, @AlejandroCornejo and I were disccusing and we didn't know which name choose. The idea was to combine two (or more) prexisting CL, where let's say the initial strain of the second law must be provided by the first, and we need to check that using the flag.

loumalouomega commented 6 years ago

Ooh, @AlejandroCornejo you already said :D

josep-m-carbonell commented 6 years ago

Ok, I think that using flags for this purpose is not the best option, you must look for a composite structure. However, the flags you propose are a different type of flags, because you want to set them in the ConstitutiveLaw itself.

loumalouomega commented 6 years ago

Well those flags are defined as local flags, we can define more local flags in our laws, but that would increase the memory required by the law. taking the already existing local flags can solve that issue.

AlejandroCornejo commented 6 years ago

mm thank you @josep-m-carbonell , which would be a better option?

josep-m-carbonell commented 6 years ago

You are welcome @AlejandroCornejo, as I said you before, I think that what you try to do can be achieved with a composite structure. The simpler one would be a law composed by two laws, one can be the 'master' and other can be the 'slave'. With certain rules to execute the methods. It is only an idea. If you want to implement something like this in the ConstitutiveModelsApplication, we can talk about the details for a good implementation.

AlejandroCornejo commented 6 years ago

a law composed by two laws, one can be the 'master' and other can be the 'slave' This is what we have done exactly! :) the important thing is that those "slave" CL should work also as "master" when working on its own... How do we achieve this "double life" without the flag procedure?

AlejandroCornejo commented 6 years ago

@RiccardoRossi

philbucher commented 6 years ago

@frastellini this might be related to #2414

frastellini commented 6 years ago

Thanks @philbucher for mentioning me. Yes. They are in close relation with the the sub-properties implementation. If any property should have sub-properties, we may say the first is the "master" CL and the second the "slave" CL. Maybe the sub-properties scheme solves the issue of master-slave from the subproperties block construction.

Probably @AlejandroCornejo needs an additional flag for some reason. Probably he wants to allow a kind of recursivity inside a single CL. I do not know.

@AlejandroCornejo , would the sub-property scheme solve what you need? We can talk in person, if you want, to define a proposal of solution for Kratos, probably using sub-properties... (#2414)

AlejandroCornejo commented 6 years ago

sorry for not having included you @frastellini but i thouht that was an easy issue hehe. The objective of this flag is as follows:

imagen

If this plastic law is master, then calculate the stress predictor as S = C (E - Ep) but if its slave this predictor is computed by the other slave law so it only need to get the value by using the rValues.GetStressVector()

frastellini commented 6 years ago

No problem. I agree you should use a flag named "DEPENDENT" or "SLAVE" if you think you need so.

Aditionally, @loumalouomega and me are working on a proposal for subpropeties. We will let you know soon...

RiccardoRossi commented 6 years ago

guys, i think there are two levels of discussion here:

on one side i also do not understand why the local flags are repeated. I do appreciate that they can be used differently in a different context, however is this documented somewhere. If not at least please document it in the ConstitutiveLaw documentation in the wiki? because it definitely does look a dangerous habit to me...

regarding the need of the MASTER and SLAVE flags on the other hand i am definitely more conservative. A CL should always behave the same...the one in charge of the combination should take care of this sort of things (and no flags shall be needed for that)

loumalouomega commented 6 years ago

In that case we need to discuss a proper workflow of the combined/complex CLs.

josep-m-carbonell commented 6 years ago

There are some developers with dangerous habits..? Interesting words.... Who documented the CL in the github wiki... ? And in the previous wiki..? Has this guy and others who discuss in this issue read the comments of the constitutive_law.h?

// License: BSD License // Kratos default license: kratos/license.txt // // Main authors: Riccardo Rossi // Janosch Stascheit // Nelson Maireni Lafontaine // Josep Maria Carbonell /**

Counted pointer of ConstitutiveLaw */ KRATOS_CLASS_POINTER_DEFINITION(ConstitutiveLaw);

/**
Flags related to the Parameters of the Contitutive Law */ KRATOS_DEFINE_LOCAL_FLAG( USE_ELEMENT_PROVIDED_STRAIN ); KRATOS_DEFINE_LOCAL_FLAG( COMPUTE_STRESS ); KRATOS_DEFINE_LOCAL_FLAG( COMPUTE_CONSTITUTIVE_TENSOR ); KRATOS_DEFINE_LOCAL_FLAG( COMPUTE_STRAIN_ENERGY );

KRATOS_DEFINE_LOCAL_FLAG( ISOCHORIC_TENSOR_ONLY ); KRATOS_DEFINE_LOCAL_FLAG( VOLUMETRIC_TENSOR_ONLY );

KRATOS_DEFINE_LOCAL_FLAG( MECHANICAL_RESPONSE_ONLY ); KRATOS_DEFINE_LOCAL_FLAG( THERMAL_RESPONSE_ONLY );

KRATOS_DEFINE_LOCAL_FLAG( INCREMENTAL_STRAIN_MEASURE );

///the next two flags are designed for internal use within the constitutive law. ///please DO NOT use them from the API KRATOS_DEFINE_LOCAL_FLAG( INITIALIZE_MATERIAL_RESPONSE ); KRATOS_DEFINE_LOCAL_FLAG( FINALIZE_MATERIAL_RESPONSE );

/**
Flags related to the Features of the Contitutive Law */

KRATOS_DEFINE_LOCAL_FLAG( FINITE_STRAINS ); KRATOS_DEFINE_LOCAL_FLAG( INFINITESIMAL_STRAINS );

KRATOS_DEFINE_LOCAL_FLAG( THREE_DIMENSIONAL_LAW ); KRATOS_DEFINE_LOCAL_FLAG( PLANE_STRAIN_LAW ); KRATOS_DEFINE_LOCAL_FLAG( PLANE_STRESS_LAW ); KRATOS_DEFINE_LOCAL_FLAG( AXISYMMETRIC_LAW );

KRATOS_DEFINE_LOCAL_FLAG( U_P_LAW );

KRATOS_DEFINE_LOCAL_FLAG( ISOTROPIC ); KRATOS_DEFINE_LOCAL_FLAG( ANISOTROPIC ); ...

maceligueta commented 6 years ago

Just to avoid misunderstandings, I think that when @RiccardoRossi and @josep-m-carbonell talk about a composite (or combined) CL, they are referring to an object that saves two pointers to two different CLs. Then, the methods of the composite CL fix whether they call to the first simple CL or the second. I think that the same could be achieved with templates instead of pointers. I also believe that this option is better than setting and checking flags.

loumalouomega commented 6 years ago

@josep-m-carbonell It is very difficult to follow what do you really meant to say. Remember we are expressing here via purely written media, irony and sarcasms are very difficult to follow, and depends on the cultural background. I would keep this kind of commentaries out of here to avoid misunderstandings. As well as avoiding personal attacks to avoid internal fight that drive us to no place at all, and all the problems persist.

I don't know who documented the CL in the previous wiki, we can migrate that and update it. But document is complicated and requires continuous work. The current Doxygen documentation it is currently not exported, so it requires to look directly in the code and this situation is not ideal. I would like to make this properly with @roigcarlo (we don't like the current Doxygen style, so we were looking an external utility to create a better looking doc).

Plus, our concern with the local flags is that even it this flags are not used simultaneously to use the same id is very dangerous and error prone.

AlejandroCornejo commented 6 years ago

I would like to redirect the attention to the main theme of this issue (and void those " internal fights") :

Is it dangerous to have different FLAGS with the same Id (even if migth not be used simultaneously)?
Is it a good idea to add a new flag or we think another way of doing it? (if the flag could not be added I would appreciate some help to solve the problem)

Thank you all

josep-m-carbonell commented 6 years ago

@loumalouomega, what are you talking about, sarcasm? Did you feel attacked by somebody? What do you mean when you say "our concern", are you expressing your opinion or the opinion of someone else? Who documented the CL? , I guess that maybe you and your director... But I don't know. I think that you and @roigcarlo are doing a very good job in documenting Kratos. I am not the one who said: why this was not documented... However, if you don't like something (like the flags numbering) you can change it and improve it. Do you want to put a unique flag id? Ok, It will be easier to make use of the meaning of the flags in tricky manner.. It has happened before. If you need a MASTER flag and you don't have it you will take another one....I'm sure you already did it.

I don't know why you focus on flags when people is saying to you that a composite law does not need it.

@AlejandroCornejo, you want to focus on your problem and you opened an issue that is not really defining your problem. You don't want conflicts but, in this case you started one.

loumalouomega commented 6 years ago

"There are some developers with dangerous habits..? Interesting words.... Who documented the CL in the github wiki... ? And in the previous wiki..? Has this guy and others who discuss in this issue read the comments of the constitutive_law.h?"

These words have several punctuations that can suggest irony, like is shown here: https://en.wikipedia.org/wiki/Irony_punctuation

AlejandroCornejo commented 6 years ago

mm I think that my first comment was clear Me and @loumalouomega have been thinking to add a new CL::Flag named "IS_SLAVE" to be used when a certain CL is controled by another CL, Is it possible? The mentioned inconsistency can be seen in the following lines of the constitutive_law.cpp:

"I don't know why you focus on flags when people is saying to you that a composite law does not need it." this is why I'm asking people about alternative ways of doing it!

Anyway, as we usually say when something becomes not productive: el último que cierre...

KratosMultiphysics / Kratos

[Core][ConstitutiveLaw]Inconsistency with CL::Flags #2909