[Core] Add saving distance in skin in CalculateNodalDistanceToSkinProcess

[loumalouomega]

📝 Description

This PR introduces several enhancements and adjustments in CalculateNodalDistanceToSkinProcess (the process for calculating nodal distances to the skin). Key updates include:

• Distance value storage in skin model: Implemented functionality to store distance values directly in the skin model part. By default is deactivated, if activated will save the maximum distance in the found skin geometry and set the flag VISITED. This also deactivate shared memory parallelization.

• Lambda function updates: Updated lambda functions now support the improved mechanism for saving distance values in the skin model part, facilitating a more streamlined and efficient process.

• Default parameters: Added and refined default parameters for the process to enhance user experience by providing clearer guidelines and reducing the need for manual adjustments.

• Improved printed information: Adjustments to the information printout provide a better representation of process settings and variables, aiding in debugging and process optimization.

• Documentation updates: The calculate_nodal_distance_to_skin_process.md documentation has been extensively updated to include detailed explanations of the process enhancements. It covers the support for calculating distances to skin entities, introduces new lambda functions, and discusses the addition of flags for data type selection and decision-making regarding the storage of distances in the skin model part. The documentation also clarifies the functionalities of default parameters.

• New test method: Introduced a new test method, test_ComputeDistanceToSkinWithSavedDistanceInSkin, to the test_calculate_nodal_distance_to_skin_process.py file. This test verifies that specific conditions in the skin model part are correctly addressed during the process, ensuring the reliability of the new features.

🆕 Changelog

• Added local flags for enhanced control and clarity, facilitating better customization of the process (Commit Link).

• Implemented functionality for saving maximum distance values to skin entities within the skin model part, optimizing the process by eliminating the need for parallel algorithms in this context (Commit Link).

• Added a new test case, test_ComputeDistanceToSkinWithSavedDistanceInSkin, to ensure the process accurately handles conditions within the skin model part, thereby verifying the enhancements made (Commit Link).

[rubenzorrilla]

Considering that we have a parameters-based constructor, which is the advantage of using local flags rather than a simple bool? In this regard, I note that local flags have always been a pain in the *ss.

[loumalouomega]

Considering that we have a parameters-based constructor, which is the advantage of using local flags rather than a simple bool? In this regard, I note that local flags have always been a pain in the *ss.

• Process is already a class derived from Flags, so it is already there not adding anything to memory.
• Faster.
• Less memory consumption.
• It is already done, we had a discussion back in the time of from_json_check_result_process implementation and we agreed that if more than one bool it was better this approach than adding more member variables. See

  Kratos/kratos/processes/from_json_check_result_process.h

  Line 66 in 4e44cbc

  KRATOS_DEFINE_LOCAL_FLAG( CORRECT_RESULT ); /// This flag is used in order to check that the result is correct

• Local flags are painful if you plan to use it outside the class, that is not the case.

[rubenzorrilla]

Considering that we have a parameters-based constructor, which is the advantage of using local flags rather than a simple bool? In this regard, I note that local flags have always been a pain in the *ss.

* Process is already a  class derived from Flags, so it is already there not adding anything to memory.

I think this is a legacy reason. Honestly, I don't think it is the best of the ideas to make everything derive from Flags. Having said this, I'm of the opinion of stop exploiting this capability.

* Faster.

* Less memory consumption.

Well, I don't think performance of having two bool member variables is relevant considering the sort of operations we're doing in here. Indeed, I'd consider it negligible.

* It is already done, we had  a discussion back in the time of `from_json_check_result_process` implementation and we agreed that if more than one bool it was better this approach than adding more member variables. See https://github.com/KratosMultiphysics/Kratos/blob/4e44cbc551cfd2b7a9a172eed28eaf8b678ace92/kratos/processes/from_json_check_result_process.h#L66

If so, I think we should reconsider this again @KratosMultiphysics/technical-committee .

* Local flags are painful if you plan to use it outside the class, that is not the case.

You are not planning to do so but by adding them you open the possibility for others to do it. Hence, I'm of the opinion of simply precluding it.

[loumalouomega]

Considering that we have a parameters-based constructor, which is the advantage of using local flags rather than a simple bool? In this regard, I note that local flags have always been a pain in the *ss.

* Process is already a  class derived from Flags, so it is already there not adding anything to memory.

I think this is a legacy reason. Honestly, I don't think it is the best of the ideas to make everything derive from Flags. Having said this, I'm of the opinion of stop exploiting this capability.

* Faster.

* Less memory consumption.

Well, I don't think performance of having two bool member variables is relevant considering the sort of operations we're doing in here. Indeed, I'd consider it negligible.

* It is already done, we had  a discussion back in the time of `from_json_check_result_process` implementation and we agreed that if more than one bool it was better this approach than adding more member variables. See https://github.com/KratosMultiphysics/Kratos/blob/4e44cbc551cfd2b7a9a172eed28eaf8b678ace92/kratos/processes/from_json_check_result_process.h#L66

If so, I think we should reconsider this again @KratosMultiphysics/technical-committee .

* Local flags are painful if you plan to use it outside the class, that is not the case.

You are not planning to do so but by adding them you open the possibility for others to do it. Hence, I'm of the opinion of simply precluding it.

TLDR: I removed the local flags

[pooyan-dadvand]

I tend to agree with @rubenzorrilla that making the constructor incompatible with (model,parameter) API in this case is not well justified. The process is very complex, needs lot of memory for bins and others and is very time consuming. So conversion of parameters to an internal bool is not significant.

[pooyan-dadvand]

Just to add that local flags are suitable as nodal or elemental data (where the memory and access time are crucial) but in processes it is more a legacy from the time we didn't have parameters.

[loumalouomega]

Just to add that local flags are suitable as nodal or elemental data (where the memory and access time are crucial) but in processes it is more a legacy from the time we didn't have parameters.

OK, but there are used in different places

[loumalouomega]

I tend to agree with @rubenzorrilla that making the constructor incompatible with (model,parameter) API in this case is not well justified. The process is very complex, needs lot of memory for bins and others and is very time consuming. So conversion of parameters to an internal bool is not significant.

I am using (model,parameter), I think some information was mixed

[pooyan-dadvand]

Regarding to usage of local_flags in the processes, @KratosMultiphysics/technical-committee would encourage the use of Parameters in the argument list and not using Flags nor bools if possible. Internally, we see using member variables is more clear and straightforward.

[loumalouomega]

Regarding to usage of local_flags in the processes, @KratosMultiphysics/technical-committee would encourage the use of Parameters in the argument list and not using Flags nor bools if possible. Internally, we see using member variables is more clear and straightforward.

The model part constructor is only accesible via C++, not python, do you want I removed it?, this was added less thana a week ago.

[loumalouomega]

Regarding to usage of local_flags in the processes, @KratosMultiphysics/technical-committee would encourage the use of Parameters in the argument list and not using Flags nor bools if possible. Internally, we see using member variables is more clear and straightforward.

I have nveer used local flags in this PR as input argument, I was using them to store the information (Process derives from Flags)

[pooyan-dadvand]

Are you using the non standard constructor? If not, I would remove it

[loumalouomega]

Suggestions applied @pooyan-dadvand

[loumalouomega]

Suggestions applied @pooyan-dadvand

This is ready @pooyan-dadvand

[loumalouomega]

Suggestions applied @pooyan-dadvand

This is ready @pooyan-dadvand

Ping @pooyan-dadvand

[loumalouomega]

Suggestions applied @pooyan-dadvand

This is ready @pooyan-dadvand

Ping @pooyan-dadvand

👋

[loumalouomega]

ANyone?

[loumalouomega]

@pooyan-dadvand

[loumalouomega]

Ping @pooyan-dadvand

[pooyan-dadvand]

Thanks @loumalouomega