[Core] Making Variable and VariableData constexpr by sunethwarna · Pull Request #14260 · KratosMultiphysics/Kratos

sunethwarna · 2026-03-06T08:30:52Z

📝 Description
As in the title, this PR tries to make the Variable and VariableData constexpr. One advantage is that we can now use switch jump table optimizations with variables.

This makes Variable::mZero static, which, anyway, was always default-initialized. No custom zero definitions were found anywhere in Kratos [ Check #14253 ].

Closes #14253

Eg:

Kratos/kratos/tests/cpp_tests/containers/test_variables.cpp

Lines 95 to 101 in 4eff549

    
           const auto& func_variable = [](const Variable<double>& rVariable) -> int { 
        
               switch (rVariable.Key()) { 
        
                   case PRESSURE.Key(): return 1; 
        
                   case DENSITY.Key(): return 2; 
        
                   default: return 3; 
        
               } 
        
           };

or

Kratos/kratos/tests/cpp_tests/containers/test_variables.cpp

Lines 107 to 113 in 4eff549

    
           const auto& func_variable_data = [](const VariableData& rVariableData) -> int { 
        
               switch (rVariableData.Key()) { 
        
                   case VELOCITY.Key(): return 1; 
        
                   case ACCELERATION.Key(): return 2; 
        
                   default: return 3; 
        
               } 
        
           };

Lets see whether what problems the CI gives.

🆕 Changelog

Made all the uses of Variable and VariableData to be const correct.
Made Variable::mZero static and default initialized to zero.
Made the Variable and VariableData constexpr
Added a unit test with the switch

roigcarlo · 2026-03-06T09:06:00Z

Took a first look and unless I missed something this looks fine. I focused on checking that moving from external to static inline kept the functionality unchanged. This also improves the safety of the resulting code as it delegates the responsibility of merging the definitions of the variable to the compiler, and not to the runtime.

gj @sunethwarna !!

loumalouomega

LGTM

avdg81

On behalf of @KratosMultiphysics/geomechanics, we're okay with the changes proposed for the GeoMechanicsApplication 👍

RiccardoRossi

@pooyan-dadvand is taking this over for a detailed review and approval

RiccardoRossi · 2026-03-09T10:19:11Z

        const int neigh_color = *(neighbor_colors_iterator++);
        array_1d<double,3> dist = cell_center - rConditionCenter;
        double dist_modulus = norm_2(dist);
-        array_1d<double,3> unit_dist;


why this change is needed?

It is because, this variable may be uninitialized because of the following if condition.

https://github.com/KratosMultiphysics/Kratos/blob/master/kratos%2Fmodeler%2Foperation%2Ffind_contacts_in_skin_model_part.cpp#L180-L183

unrelated then?

related, since this PR is triggering the warning.... [For some reason it was not triggered earlier, but the warning makes sense]

RiccardoRossi · 2026-03-09T10:26:39Z

-        const VariableType* pTimeDerivativeVariable = nullptr
-        )
-        : VariableData(NewName, sizeof(TDataType)),
-          mZero(Zero),


the problem is that ublas does not allow constexpr assignement?

Not for the types which are dynamic.... Like Vector and Matrix.

roigcarlo · 2026-03-09T10:29:09Z

 // Application includes
 #include "includes/variables.h"
 #include "includes/kratos_flags.h"
+#include "includes/cfd_variables.h"


Is this needed?

Yes, the FACTIONAL_STEP variable was defined both in variables.h and cfd_variables.h... since we now doing inline constexpr, there can only be one definition in one TU. So I removed the definition of the variable from variables.h. so now, this element uses it, so I had to include it.

I think this is an advantage! Nevertheless, this leads me to a very important question:

What about having the same variable in two different application?

This is the nice thing. If the variable is in the same TU, then, it will have the same memory location, key (given the name is the same) and the name. But if they are in different TU, they will have different memory location but with the same key (as long as the name is the same) and the key same... So, now no problems in redefining the variable in multiple applications or TUs.... :)

This is the reason why i put in the pybind the py::eq and __hash__ function definitions. earlier it was using the memory locations to check for equality and hashing in python, now it uses the var key. Which i think is better and proper.

pooyan-dadvand

While I support the idea behind this PR, I am not sure how it affects the variables defined in different apps. Is it possible to add test for that?

pooyan-dadvand · 2026-03-16T09:02:05Z

    KRATOS_DEFINE_APPLICATION_VARIABLE( MPM_APPLICATION, double, MP_KINETIC_ENERGY )
    KRATOS_DEFINE_APPLICATION_VARIABLE( MPM_APPLICATION, double, MP_STRAIN_ENERGY )
    KRATOS_DEFINE_APPLICATION_VARIABLE( MPM_APPLICATION, double, MP_TOTAL_ENERGY )
-    KRATOS_DEFINE_APPLICATION_VARIABLE( MPM_APPLICATION, double, MP_TEMPERATURE)


Is this related to this PR?

yes, this was a redeclaration of a variable defined in line 45

pooyan-dadvand · 2026-03-16T09:02:32Z

-	KRATOS_DEFINE_APPLICATION_VARIABLE(OPTIMIZATION_APPLICATION, double, D_MAX_STRESS_D_PT);
-    KRATOS_DEFINE_APPLICATION_VARIABLE(OPTIMIZATION_APPLICATION, double, D_MAX_STRESS_D_CT);
-	KRATOS_DEFINE_APPLICATION_VARIABLE(OPTIMIZATION_APPLICATION, double, D_MAX_STRESS_D_PD);
-    KRATOS_DEFINE_APPLICATION_VARIABLE(OPTIMIZATION_APPLICATION, double, D_MAX_STRESS_D_CD);


Is this related to this PR?

ok, a short summary

KRATOS_DEFINE_APPLICATION_VARIABLE used to declare a variable. Now it actually defines one.

What you're seeing here are all variables that have duplicate declarations, which have now become duplicate definitions, which is of course an error.

pooyan-dadvand · 2026-03-16T09:03:39Z

-
-    KRATOS_CREATE_VARIABLE_WITH_TIME_DERIVATIVE(double, TURBULENT_ENERGY_DISSIPATION_RATE_2, RANS_AUXILIARY_VARIABLE_2)
-    KRATOS_CREATE_VARIABLE_WITH_TIME_DERIVATIVE(double, TURBULENT_ENERGY_DISSIPATION_RATE, TURBULENT_ENERGY_DISSIPATION_RATE_2)
-


Unrelated changes?

explained in the above comment.

pooyan-dadvand · 2026-03-16T09:03:47Z

    // k-omega turbulence modelling specific additional variables
-    KRATOS_CREATE_VARIABLE_WITH_TIME_DERIVATIVE(double, TURBULENT_SPECIFIC_ENERGY_DISSIPATION_RATE_2, RANS_AUXILIARY_VARIABLE_2)
-    KRATOS_CREATE_VARIABLE_WITH_TIME_DERIVATIVE(double, TURBULENT_SPECIFIC_ENERGY_DISSIPATION_RATE, TURBULENT_SPECIFIC_ENERGY_DISSIPATION_RATE_2)
-


Unrelated changes?

explained in the above comment

pooyan-dadvand · 2026-03-16T09:04:13Z

-    KRATOS_DEFINE_APPLICATION_VARIABLE( RANS_APPLICATION, double, TURBULENT_KINETIC_ENERGY )
-    KRATOS_DEFINE_APPLICATION_VARIABLE( RANS_APPLICATION, double, TURBULENT_ENERGY_DISSIPATION_RATE )
-    KRATOS_DEFINE_APPLICATION_VARIABLE( RANS_APPLICATION, double, TURBULENT_KINETIC_ENERGY_RATE )
-    KRATOS_DEFINE_APPLICATION_VARIABLE( RANS_APPLICATION, double, TURBULENT_ENERGY_DISSIPATION_RATE_2 )


xplained in the above comment

pooyan-dadvand · 2026-03-16T17:58:08Z

    KRATOS_DEFINE_3D_VARIABLE_WITH_COMPONENTS( REACTION_MOMENT )
-    KRATOS_DEFINE_3D_VARIABLE_WITH_COMPONENTS( ANGULAR_VELOCITY )
    KRATOS_DEFINE_3D_VARIABLE_WITH_COMPONENTS( ANGULAR_ACCELERATION )
+    KRATOS_DEFINE_3D_VARIABLE_WITH_COMPONENTS_WITH_TIME_DERIVATIVE(ANGULAR_VELOCITY, ANGULAR_ACCELERATION)


not a cosmetic change. Ealier it was possible to define a variable with extern and pass the time derivatives with the proper definition with another macro. Now since it is inline static constexpr, the whole definition and declaration should be in once place [In my opinion it is cleaner). Therefore, when you want to define a var with time derivatives, you should do it at once, hence the above change.

pooyan-dadvand · 2026-03-16T17:59:03Z

        const int neigh_color = *(neighbor_colors_iterator++);
        array_1d<double,3> dist = cell_center - rConditionCenter;
        double dist_modulus = norm_2(dist);
-        array_1d<double,3> unit_dist;


unrelated then?

pooyan-dadvand · 2026-03-16T17:59:57Z

    .def("GetComponentIndex", &VariableData::GetComponentIndex)
    .def("IsComponent", &VariableData::IsComponent)
    .def("__str__", PrintObject<VariableData>)
+    .def("__hash__", &VariableData::Key)


Intersting!

The both py::eq and __hash__ was implemented to allow comparing variables correctly even if they come from different TUs (different applications). Earlier, the memory location was used for hashing [ luckily, the unit there was one unit test, and it was comparing the memory locations for hashing ]. Now this one make sure that we always comapre and hash based on the var key.

sunethwarna · 2026-03-17T06:58:01Z

While I support the idea behind this PR, I am not sure how it affects the variables defined in different apps. Is it possible to add test for that?

So the nice thing about the change is, now all the variables which have the same name will have the same key and same memory location if they are in the same TU. But if they are redefined in a different TU again such as in an application, they will have a different memory location, but the key and the name will be the same. In C++ we have the operator== defined for VariableData which is used by Variable as well. So no problem in that with different TUs defining the same variable. For python have added following to mimic the same behavior.

    .def("__hash__", &VariableData::Key)
    .def(py::self == py::self)
    .def(py::self != py::self)

A unit test is added to check the intended behavior in a scenario where the same variable is deifned from multiple TUs (or multiple apps)

We have addressed all of your comments @pooyan-dadvand . Could you please have a look again? Thanks... :D

…constexpr

sunethwarna added 8 commits March 6, 2026 06:56

Variable const correctness attempt - 1

89fd07d

make VariableData constexpr

4c9cc49

make variable constexpr

83bf4a1

update serializer

b7c4708

update variables.h/cpp - 1

6b07347

add a test for switch

c9d3046

add time derivatives

59103fb

Variable const correctness attempt - 2

4eff549

sunethwarna self-assigned this Mar 6, 2026

sunethwarna added DO NOT MERGE C++ Feature Kratos Core Applications labels Mar 6, 2026

sunethwarna added this to Technical Commiittee Mar 6, 2026

sunethwarna moved this to 👀 Next meeting TODO in Technical Commiittee Mar 6, 2026

sunethwarna requested review from matekelemen and roigcarlo March 6, 2026 08:32

minor

aca4ffa

try for windows - 1

b428296

sunethwarna marked this pull request as ready for review March 6, 2026 09:59

sunethwarna requested review from a team as code owners March 6, 2026 09:59

sunethwarna requested a review from RiccardoRossi March 6, 2026 10:02

sunethwarna requested a review from loumalouomega March 6, 2026 10:03

loumalouomega added 2 commits March 6, 2026 11:08

Style

27a88e6

More style

63dd423

loumalouomega reviewed Mar 6, 2026

View reviewed changes

redirect redundant macros

5e186d2

sunethwarna added Enhancement and removed DO NOT MERGE labels Mar 6, 2026

avdg81 reviewed Mar 6, 2026

View reviewed changes

sunethwarna and others added 3 commits March 6, 2026 13:15

add py::eq to VariableData

3979bc6

add hash

c2d1c22

try for windows - 2

1af8a1d

RiccardoRossi reviewed Mar 9, 2026

View reviewed changes

roigcarlo reviewed Mar 9, 2026

View reviewed changes

pooyan-dadvand requested changes Mar 16, 2026

View reviewed changes

sunethwarna added 3 commits March 17, 2026 07:42

rename the macros with Kratos

d07a179

revert Flag changes

5a7c5d3

revert unregistration

d8b3b31

sunethwarna and others added 7 commits March 17, 2026 08:03

re define the commented vars

77a515f

minor

9f89af3

add geometrical_object.h to variables.h

5ab8abf

add a test with different TUs having the same variable defined.

df7ff98

Merge branch 'master' into core/make_variable_constexpr

10f40c1

Merge remote-tracking branch 'origin/master' into core/make_variable_…

e31e543

…constexpr

restricting the ctors

7992683

matekelemen requested changes Mar 26, 2026

View reviewed changes

Comment thread kratos/containers/variable_data.h Outdated

Comment thread kratos/containers/variable.h Outdated

sunethwarna added 4 commits March 26, 2026 13:11

removing serialization of vars

90e459c

revert data serialization

4869a6b

revert the concepts

a27cb96

remove the test

d731b3a

	const auto& func_variable = [](const Variable<double>& rVariable) -> int {
	switch (rVariable.Key()) {
	case PRESSURE.Key(): return 1;
	case DENSITY.Key(): return 2;
	default: return 3;
	}
	};

	const auto& func_variable_data = [](const VariableData& rVariableData) -> int {
	switch (rVariableData.Key()) {
	case VELOCITY.Key(): return 1;
	case ACCELERATION.Key(): return 2;
	default: return 3;
	}
	};


		KRATOS_CREATE_VARIABLE_WITH_TIME_DERIVATIVE(double, TURBULENT_ENERGY_DISSIPATION_RATE_2, RANS_AUXILIARY_VARIABLE_2)
		KRATOS_CREATE_VARIABLE_WITH_TIME_DERIVATIVE(double, TURBULENT_ENERGY_DISSIPATION_RATE, TURBULENT_ENERGY_DISSIPATION_RATE_2)

Conversation

sunethwarna commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roigcarlo commented Mar 6, 2026

Uh oh!

loumalouomega left a comment

Choose a reason for hiding this comment

Uh oh!

avdg81 left a comment

Choose a reason for hiding this comment

Uh oh!

RiccardoRossi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunethwarna Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pooyan-dadvand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sunethwarna commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

sunethwarna commented Mar 6, 2026 •

edited

Loading

sunethwarna Mar 9, 2026 •

edited

Loading

sunethwarna commented Mar 17, 2026 •

edited

Loading