Proposal: AddNodalVariables type method for Processes.

[jcotela]

Hi everyone,
The discussion in #854 made me think that it would be nice if the process class had some way of adding its required variables to the model part, in order to make it more robust. The idea is as follows: now it is the job of the Python solver to add all required variables to the nodes, but sometimes processes use additional variables in their regular operation. I think it would be desirable that each process had the opportunity to add its required variables to the modelpart (not only check if they have been added by the solver). In my view, this would help us

• Increase robustness: The process ensures the model part is correctly prepared for the operations to be done within the process.
• Reduce memory usage: "niche" variables, used only within a process and not within the main solution loop are only added when the process is used.

There are some potential issues, as @rubenzorrilla was mentioning in #854, with submodelparts, but I don't think they would be a blocker.

Does anyone else think this would be good to have/sees problems with this idea?

[loumalouomega]

Something important in order to reduce memory usage is to reduce the number of historical variables in processes and utilities, many processes keep using by default the historical variables

[rubenzorrilla]

(Just to recall the potential issue discussed in #854)

The issue in here is what happens if the process is constructed using a submodelpart (a common operation) instead of using the main model part, what is to say that some variables are added only in a set of nodes instead of adding them to all the nodes.
On the other hand, I completely agree with the advantages pointed by @jcotela.

[RiccardoRossi]

I Line the idea, but as i recall there is a problem with the order of creation of the processes. Still it is a +1 If we can do it correctly Riccardo

…

On Oct 19, 2017 4:45 PM, "jcotela" ***@***.***> wrote: Hi everyone, The discussion in #854 <#854> made me think that it would be nice if the process class had some way of adding its required variables to the model part, in order to make it more robust. The idea is as follows: now it is the job of the Python solver to add all required variables to the nodes, but sometimes processes use additional variables in their regular operation. I think it would be desirable that each process had the opportunity to add its required variables to the modelpart (not only check if they have been added by the solver). In my view, this would help us - Increase robustness: The process ensures the model part is correctly prepared for the operations to be done within the process. - Reduce memory usage: "niche" variables, used only within a process and not within the main solution loop are only added when the process is used. There are some potential issues, as @rubenzorrilla <https://github.com/rubenzorrilla> was mentioning in #854 <#854>, with submodelparts, but I don't think they would be a blocker. Does anyone else think this would be good to have/sees problems with this idea? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#869>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHr7EaKdcXB-ZAl2ubdP1P0ZEOxxoSPSks5st2CBgaJpZM4P_WF9> .

[philbucher]

I also like the idea but I want to draw your attention to a potential problem:
If variables are added to the ModelPart after a certain point, it creates random segfaults. (see #492)
Therefore if a user decides to call a process later for whatever reason, this might be a problem

[pooyan-dadvand]

👍 for the @jcotela proposal.

I see the concern of @rubenzorrilla but at this moment all sub model parts are sharing the same variable lists with the main one. So adding a variable to any of them would add it to all. I think this mechanism would avoid problems even in adding variable by process.

[philbucher]

I thought abt this and have two comments on this:

1. In StructuralMechanics I recently introduced the possibility to add Variables that are not in the solvers, through ProjectParameters , see Structural Mechanics small enhancements in solver #1113
   This way the user can already include Variables that are needed.
   This could be an intermediate solution to things like Add Q_VALUE as variable for FluidDynamicsApplication #854
2. Regarding the processes I couldn't think of a clean solution. It seems like we have a Hen-Egg problem here: The processes are created only when the ModelPart is already read in the solver. At this point however it is too late to add Variables to the ModelPart (see the problems discussed in GiDIO segfaults without Variables added #492 )
   On the other hand creating the processes first also does not work bcs afaik they need the ModelPart at construct time (is this so?)
   A solution could be to have a @staticmethod in the python processes, that returns the Variables that are used in the process.
   This would however require that the processes have to be passed to the solver, where the evaluation if this staticmethod would be done, sth like:

AddVariables():
...
   for process in processes:
      variables = process.StaticGetVariables
      ... (Check if Var is already in ModelPart) ...
      modelpart.AddNodalSolutionStepVariable(variable)

[jcotela]

Hi @philbucher, I think that the solution is essentially in the line you are suggesting. The AddVariables methods for processes should be static. A possibility would be:

1. The solver has a constructor that receives json data as input -> At this point we can have a list of required processes (even if we don't initialize them yet).
2. The solver has an AddVariables method, which internally also calls the (static) method for processes. This is essentially your proposed function.
3. In solver.Initialize() the processes are finally constructed. We may want to also add a Check() method to the processes, which can verify that the required variables have been added. Alternatively, some processes do this kind of checking in their constructor, which would also work.

[pooyan-dadvand]

Having the static AddVariables methods is not a bad idea and maybe the best solution now. The json also make sense but pushing problem to GUI is not my favorite option. (It perfectly valid for intermediate solution as @philbucher mentioned)

Just to add as a long term solution: It is time to think about Project object...

[josep-m-carbonell]

I think that adding variables here and there can result in a big caos. I would suggest to add a method in processes to ask for the variables GetVariables(). Externally a manager asks for the variables of processes, solvers, and whatever your problem needs and it adds to the main model part.

It is true that now, when processes are build, a model (list of model parts) is passed to the constructor. Usually the modelpart where is going to apply the process is taken from the model in the constructor. The change here would be to pass an empty model and ask for the modelpart in ExecuteInitialize(), once the model would read and build.

I just want to ask if this solucion have any conflict with you current processes and solving designs?

[RiccardoRossi]

to give my two cents:

i agree on Jordi's solution, however i think it could make sense to combine with the idea of @josep-m-carbonell (i don't like the name GetVariables though)

i don't love the idea of manually adding to the .json . I think it can be done to add specific variables for special reasons but not as a general mechanism for solvers or processes to add the variables they need.

in any case i also think that all of this is compatible with the idea of a "Project".
Indeed

[maceligueta]

It looks like there's kind of an agreement: static method, json provides the variables,...
Can we write a prototype of how the code would end up? This would allow to polish the details (names of the functions, potential unexpected problems).

[RiccardoRossi]

no, i don't think that json should provide the variables. it can be an extra mechanism but not the first one. And the method should not be static, i would prefer the mechanism that Jordi proposes

…

On Fri, Dec 22, 2017 at 11:16 AM, Miguel Angel Celigueta < ***@***.***> wrote: I looks like there's kind of an agreement: static method, json provides the variables,... Can we write a prototype of how the code would end up? This would allow to polish the details (names of the functions, potential unexpected problems). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#869 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHr7EQu1LfaUcG2V-eD8JPtTQ3hOrP_xks5tC4GGgaJpZM4P_WF9> .

-- *Riccardo Rossi* PhD, Civil Engineer member of the Kratos Team: www.cimne.com/kratos Tenure Track Lecturer at Universitat Politècnica de Catalunya, BarcelonaTech (UPC) Full Research Professor at International Center for Numerical Methods in Engineering (CIMNE) C/ Gran Capità, s/n, Campus Nord UPC, Ed. B0, Despatx 102 (please deliver post to the porters of building C1) 08034 – Barcelona – Spain – www.cimne.com - T.(+34) 93 401 56 96 skype: *rougered4* <http://www.cimne.com/> <https://www.facebook.com/cimne> <http://blog.cimne.com/> <http://vimeo.com/cimne> <http://www.youtube.com/user/CIMNEvideos> <http://www.linkedin.com/company/cimne> <https://twitter.com/cimne> Les dades personals contingudes en aquest missatge són tractades amb la finalitat de mantenir el contacte professional entre CIMNE i voste. Podra exercir els drets d'accés, rectificació, cancel·lació i oposició, dirigint-se a [email protected]. La utilització de la seva adreça de correu electronic per part de CIMNE queda subjecte a les disposicions de la Llei 34/2002, de Serveis de la Societat de la Informació i el Comerç Electronic. Imprimiu aquest missatge, només si és estrictament necessari. <http://www.cimne.com/>

[maceligueta]

Then I am completely lost. I thought @jcotela used the word static and talked about passing the json to the solver (in principle to know the list of processes, but then the variables too, right?).
I mean, for a typical process like 'AssignVectorVariableProcess', the variable to assign can change from case to case, and it is written in the json only. I don't understand how will you know the variables to add, @RiccardoRossi .

[RiccardoRossi]

well, then i think that the proposal of jordi was to have a "empty constructor" and then a AddVariables and Initialize function.

we could then first construct the processes, call the AddVariables and then call the initialize function.

this is not incompatible with additionally reading variables from json and with the proposal of @josep-m-carbonell

[jcotela]

My proposal, in short, is to provide a function that adds the required variables on each process (or at least declares them, as @josep-m-carbonell is suggesting). The details of it being a static method or moving stuff away from the constructor to the initialize method, should be "whatever works best", I don't have a strong preference.
Regarding processes like the one @maceligueta is mentioning, which work on any variable, the variable will have to be provided by json (or in some other way), but this is already done now.

The point is: we have one or more solvers and a list of processes. From that, we should be able to determine the list of required variables and add them to the model part automatically.

[jcotela]

Closing due to inactivity. I don't think my original proposal can be realized in the current design (#2994 specifically requires the model part to be created before the processes). I still think the goal of not having to add nodal variables to a solver when only one (optional) process requires them would be nice to have, so feel free to reopen if anyone has new ideas.

[philbucher]

just for the record, I still have this on my todo-list
I think I have an idea, but I will first have to test it for which I need to find time :)

[maceligueta]

I also find this interesting. We talked with @gcasas about this and about Utilities (which also need variables). They should add the variables or at least make a check. How to enforce this is also an interesting discussion.

[pooyan-dadvand]

I also liked the idea... and I think sometime soon we should deal with it again...

[philbucher]

I implemented this in #3561