[Core][MeshingApplication] Extend gradient process to other geometries. Reduce the number of template arguments to reduce compilation time #3702

loumalouomega · 2018-12-19T19:27:36Z

This allows to use the gradient an Hessian with geometries different than triangles and tetras. Additionally it reduces significantly the number of template arguments required

@riccardotosi I will add the corrections you suggested on the Hessian in a future PR

philbucher

wasn't it you that introduced the template-args?

kratos/tests/cpp_tests/processes/test_compute_nodal_gradient_process.cpp

kratos/python/add_processes_to_python.cpp

applications/MeshingApplication/tests/mmg_lagrangian_test/beam3D_test_result.sol

applications/MeshingApplication/tests/mmg_lagrangian_test/beam2D_test_result.sol

applications/MeshingApplication/tests/cpp_tests/test_metric_process.cpp

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

philbucher · 2018-12-19T22:13:30Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

-        it_node->SetValue(ANISOTROPIC_RATIO, ratio);
+    const auto& it_element_begin = mThisModelPart.ElementsBegin();
+    const auto& r_first_element_geometry = it_element_begin->GetGeometry();
+    const std::size_t dimension = r_first_element_geometry.WorkingSpaceDimension();


I would say the processinfo is safer

determining the dimension of the problem

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

Co-Authored-By: loumalouomega <vicente.mataix@polytechnique.edu>

philbucher · 2018-12-20T15:58:50Z

why do you make your algorithm to only consider the first element to determine the number of node, dimension?
I had the same comment in another PR of you some days ago

This can be easily broken!

loumalouomega · 2018-12-20T16:25:33Z

Yes, you are right. This branch was created at the same time and I had not though about the repeated pieces of code. I will change that. El jue., 20 dic. 2018 16:58, Philipp Bucher <notifications@github.com> escribió:

…

why do you make your algorithm to only consider the first element to determine the number of node, dimension? I had the same comment in another PR of you some days ago This can be easily broken! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3702 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATEMgJ4o89qP-B0hDAZ8RqoTqD17U2UMks5u67O7gaJpZM4Za4VM> .

loumalouomega · 2018-12-21T17:03:15Z

Done @philbucher

philbucher

Take a look at codacy (minor)

If you use DOMAIN_SIZE to determine the dimension the you could add a check if the value is reasonable

kratos/processes/compute_nodal_gradient_process.cpp

philbucher · 2018-12-25T13:27:27Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+/***********************************************************************************/
+/***********************************************************************************/
+
+Parameters ComputeHessianSolMetricProcess::GetDefaultParameters()


applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

loumalouomega · 2018-12-25T22:42:17Z

@philbucher Comments adressed

loumalouomega · 2018-12-29T13:33:22Z

Ping @philbucher

philbucher

some comments are not addressed

plus some more minor comments

philbucher · 2018-12-29T18:54:30Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

-        it_node->SetValue(ANISOTROPIC_RATIO, ratio);
+    const auto& it_element_begin = mThisModelPart.ElementsBegin();
+    const auto& r_first_element_geometry = it_element_begin->GetGeometry();
+    const std::size_t dimension = r_first_element_geometry.WorkingSpaceDimension();


determining the dimension of the problem

philbucher · 2018-12-29T18:54:35Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+        VariableUtils().CheckVariableExists(mrOriginVariableDoubleList[0], nodes_array);
+    } else {
+        VariableUtils().CheckVariableExists(mrOriginVariableComponentsList[0], nodes_array);
+    }
    for (auto& i_node : nodes_array)
        KRATOS_ERROR_IF_NOT(i_node.Has(NODAL_H)) << "NODAL_H must be computed" << std::endl;


philbucher · 2018-12-29T18:54:53Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+    mrOriginVariableComponentsList.push_back(&rVariable);
+
+    // We check the parameters
+    Parameters default_parameters = GetDefaultParameters();


philbucher · 2018-12-29T18:54:59Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp


+    // We check the parameters
+    Parameters default_parameters = GetDefaultParameters();


RecursivelyValidateAndAssignDefaults can not work with const Parameters

indeed, I checked, even though I think it should be
=> maybe after #58

philbucher · 2018-12-29T18:55:32Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+    if (dimension == 2) {
+        CalculateMetric<2>();
+    } else {
+        CalculateMetric<3>();


else if dim == 3

else: Error

philbucher · 2018-12-29T18:56:42Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+/***********************************************************************************/
+/***********************************************************************************/
+
+Parameters ComputeHessianSolMetricProcess::GetDefaultParameters()


philbucher · 2018-12-29T18:56:58Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+/***********************************************************************************/
+/***********************************************************************************/
+
+Parameters ComputeHessianSolMetricProcess::GetDefaultParameters()


Suggested change

Parameters ComputeHessianSolMetricProcess::GetDefaultParameters()

Parameters ComputeHessianSolMetricProcess::GetDefaultParameters() const

philbucher · 2018-12-29T18:57:19Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+void ComputeHessianSolMetricProcess::InitializeVariables(Parameters ThisParameters)
+{
+    // Get default variables
+    Parameters default_parameters = GetDefaultParameters();


Cannot be const, (the function can be), but not the parameters. Maybe adding a new method to the Parameters (the one is still in the oldest open PR)

philbucher · 2018-12-29T18:57:25Z

applications/MeshingApplication/custom_processes/metrics_hessian_process.cpp

+    // Set variables
+    mMinSize = ThisParameters["minimal_size"].GetDouble();
+    mMaxSize = ThisParameters["maximal_size"].GetDouble();
+    mEnforceCurrent = ThisParameters["enforce_current"].GetBool();


Member variables, cannot be const

philbucher

looks good to me now 👍

loumalouomega added 26 commits December 2, 2018 18:22

WIP general gradient process

1e72653

Corrections

d094746

Solving problem

c2be830

Reducing one more level the templates

125b74c

Using bool

aa0bc33

Removing unused includes

f33da25

Adapting

c0552c2

Update sol files

f5703c6

Minor clean up

3f99995

Merge branch 'master' into core/extend-gradient-process

9c9d42e

Extend hessian process

ca7c2a7

Update tests

b83f5df

Merge branch 'master' into core/extend-gradient-process

0df01c8

Some fixes

5d738fb

Merge branch 'master' into core/extend-gradient-process

8c70320

Avoid conflict

e8c2e75

Merge branch 'master' into core/extend-gradient-process

e029fe8

Revert conflict

3535733

Missing

586a481

New tests

3c54ceb

Rename tests

2e75fdf

Avoid conflict

6c0124d

Merge branch 'master' into core/extend-gradient-process

9ab8ee6

Revert conflict

77c9b00

Merge branch 'master' into core/extend-gradient-process

0ad1778

Merge branch 'master' into core/extend-gradient-process

7d943ca

loumalouomega requested review from philbucher and riccardotosi December 19, 2018 19:27

loumalouomega added Enhancement Cleanup labels Dec 19, 2018

loumalouomega added Kratos Core Applications labels Dec 19, 2018

philbucher reviewed Dec 19, 2018

View reviewed changes

Missing const

8c74b17

Co-Authored-By: loumalouomega <vicente.mataix@polytechnique.edu>

loumalouomega added 2 commits December 21, 2018 12:15

Merge branch 'master' into core/extend-gradient-process

b397afe

Suggestions by @philbucher

32b2d39

Corrections

6115c6b

loumalouomega mentioned this pull request Dec 23, 2018

[MultilevelMonteCarloApplication] Added serialzation, adaptive refinement and MultilevelMonteCarlo class #3717

Merged

Last correction

31d471b

philbucher reviewed Dec 25, 2018

View reviewed changes

loumalouomega and others added 4 commits December 25, 2018 20:17

Domain size

3cb7d6e

DOMAIN_SIZE

ad25195

Trailing whitespace

07bf448

Setting DOMAIN_SIZE on tests

aafd81e

loumalouomega and others added 3 commits December 26, 2018 11:43

Some suggestions

bfb4b25

Removing commented line

93a7a22

Clean up

39adc4c

philbucher reviewed Dec 29, 2018

View reviewed changes

loumalouomega added 2 commits December 30, 2018 01:47

Merge branch 'master' into core/extend-gradient-process

f70f4b3

Suggestions by @philbucher

e431150

philbucher approved these changes Dec 30, 2018

View reviewed changes

loumalouomega merged commit 12e74a8 into master Dec 30, 2018

loumalouomega deleted the core/extend-gradient-process branch December 30, 2018 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core][MeshingApplication] Extend gradient process to other geometries. Reduce the number of template arguments to reduce compilation time #3702

[Core][MeshingApplication] Extend gradient process to other geometries. Reduce the number of template arguments to reduce compilation time #3702

loumalouomega commented Dec 19, 2018

philbucher left a comment

philbucher Dec 19, 2018

loumalouomega Dec 19, 2018

philbucher Dec 29, 2018

philbucher commented Dec 20, 2018

loumalouomega commented Dec 20, 2018 via email

loumalouomega commented Dec 21, 2018

philbucher left a comment

philbucher Dec 25, 2018

philbucher Dec 29, 2018

loumalouomega commented Dec 25, 2018

loumalouomega commented Dec 29, 2018

philbucher left a comment

philbucher Dec 29, 2018

philbucher Dec 29, 2018

philbucher Dec 29, 2018

philbucher Dec 29, 2018

loumalouomega Dec 30, 2018

philbucher Dec 30, 2018

philbucher Dec 29, 2018

philbucher Dec 29, 2018

philbucher Dec 29, 2018

philbucher Dec 29, 2018

loumalouomega Dec 30, 2018

philbucher Dec 29, 2018

loumalouomega Dec 30, 2018

philbucher left a comment


		// We check the parameters
		Parameters default_parameters = GetDefaultParameters();

	Parameters ComputeHessianSolMetricProcess::GetDefaultParameters()
	Parameters ComputeHessianSolMetricProcess::GetDefaultParameters() const

[Core][MeshingApplication] Extend gradient process to other geometries. Reduce the number of template arguments to reduce compilation time #3702

[Core][MeshingApplication] Extend gradient process to other geometries. Reduce the number of template arguments to reduce compilation time #3702

Conversation

loumalouomega commented Dec 19, 2018

philbucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philbucher commented Dec 20, 2018

loumalouomega commented Dec 20, 2018 via email

loumalouomega commented Dec 21, 2018

philbucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loumalouomega commented Dec 25, 2018

loumalouomega commented Dec 29, 2018

philbucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philbucher left a comment

Choose a reason for hiding this comment