Kratos Master-Slave Constraints to Neighbours Process and Utility

[SADPR]

This pull request addresses the feedback and comments received on my previous pull request (#11083).

Main changes:

• Renamed the utility to "AssignMasterSlaveConstraintsToNeighboursUtility" to align with the Kratos Multiphysics Master-Slave naming convention.
• Updated tests and file namings to reflect the changes in the utility.
• Improved code structure by eliminating repetition for double and/or array variables. Instead, the process now generates a Python list containing the names of the variables. That is:

"variable_names" : ["DISPLACEMENT", "PRESSURE"]:

variable_names = ["DISPLACEMENT_X","DISPLACEMENT_Y","PRESSURE"] for 2D
or:
variable_names = ["DISPLACEMENT_X","DISPLACEMENT_Y","DISPLACEMENT_Z","PRESSURE"] for 3D

This approach allows the C++ code to receive the variable list and accommodates the 2D and 3D problem discussed in the previous pull request.

These changes enhance the clarity and consistency of the codebase and improve the usability of the utility. The updated tests ensure proper functionality and maintain code reliability.

I request the community to review this pull request and provide any further feedback or suggestions for improvement.

Thank you!

[loumalouomega]

Usually we consider https://en.wikipedia.org/wiki/Indentation_style#K&R_style with Linux kernel variant

[loumalouomega]

At least I consider that and I ask other to use that

[SADPR]

Will update this.

[loumalouomega]

Should be r_variable (snake case inside scope, cammel case for input)

[loumalouomega]

I didn't know about std::reference_wrapper, until now we were using variables raw pointers...

[SADPR]

@rubenzorrilla I think it is now ready to check. I hope I covered all your suggestions.

[RiccardoRossi]

minor comments

[RiccardoRossi]

this should be

noalias(rNode.FastGetSolutionStepValue(VELOCITY)) = velocity_vector

[RiccardoRossi]

please do not use the thread_local keyword.

in particular this is a stack array, so you can just define it there where you use it

[RiccardoRossi]

same

[RiccardoRossi]

define it here locally

[RiccardoRossi]

see the noalias comment above

noalias(rNode.FastGetSolutionStepValue(MESH_DISPLACEMENT)) = rotated_point - rNode.GetInitialPosition();

[RiccardoRossi]

it would be better if you pass the local_results in the argument, making it a TLS variable if it is in a parallel loop

[SADPR]

@loumalouomega if you can, please also take a look to this one. I implemented all suggestions.

[loumalouomega]

👀

[loumalouomega]

My only concern would be to remove the old proceess, ia priori if in the Core is part of the API and in theory maybe someone else is using it

(I also have aestheic comments, but I am not going to be that "tocah****")

[SADPR]

My only concern would be to remove the old proceess, ia priori if in the Core is part of the API and in theory maybe someone else is using it

(I also have aestheic comments, but I am not going to be that "tocah****")

We renamed "AssignMPCsToNeighboursUtility" to "AssignMasterSlaveConstraintsToNeighboursUtility" because while "MPC" is a general term, the master-slave constraint is a specific type of MPC used in Kratos. We believe this new name provides a more accurate description. To ensure backward compatibility, we can provide an alias for the old process name that points to the new one.

[SADPR]

My only concern would be to remove the old proceess, ia priori if in the Core is part of the API and in theory maybe someone else is using it

(I also have aestheic comments, but I am not going to be that "tocah****")

Here's my proposal to ensure we don't disrupt users relying on the old process name or settings:

Process Name: I've retained the file assign_mpcs_to_neighbours_process.py. If someone uses the old AssignMPCsToNeighboursProcess, it'll still work but will throw a deprecation warning, nudging them towards the new name.

Settings: For those using the old setting assign_mpcs_every_time_step, it'll be automatically swapped with the new reform_constraints_at_each_step, accompanied by a deprecation warning.

I believe this approach ensures a smooth transition without breaking anyone's workflow.

What do you think?

Here's the code for the proposed changes (assign_mpcs_to_neighbours_process.py):

import KratosMultiphysics 
from KratosMultiphysics.assign_master_slave_constraints_to_neighbours_process import AssignMasterSlaveConstraintsToNeighboursProcess

def Factory(settings, model):
    KratosMultiphysics.Logger.PrintWarning("assign_mpcs_to_neighbours_process", "This process is deprecated. Please use 'assign_master_slave_constraints_to_neighbours_process' instead.")
    return AssignMPCsToNeighboursProcess(model, settings["Parameters"])

class AssignMPCsToNeighboursProcess(AssignMasterSlaveConstraintsToNeighboursProcess):
    def __init__(self, model, settings):
        # The setting 'assign_mpcs_every_time_step' was renamed to 'reform_constraints_at_each_step'
        # This block ensures backward compatibility by updating the old setting to the new one and issuing a warning.
        if settings.Has("assign_mpcs_every_time_step"):
            KratosMultiphysics.Logger.PrintWarning("AssignMPCsToNeighboursProcess", "'assign_mpcs_every_time_step' is deprecated. Please use 'reform_constraints_at_each_step' instead.")
            settings.AddValue("reform_constraints_at_each_step", settings["assign_mpcs_every_time_step"])
            settings.RemoveValue("assign_mpcs_every_time_step")
        
        super().__init__(model, settings)