[Core][NURBS] nurbs geometry #5361

tteschemacher · 2019-08-06T09:31:49Z

Hi all,

I am adding here the geometries for pure NURBS entitities and the necessary utiltity functions.

Those are ported from the very good work of @oberbichler with ANurbs and #5344. We adapted it to fully fit to the KRATOS style.

loumalouomega · 2019-08-06T09:37:33Z

kratos/geometries/nurbs_curve_geometry.h

+    ///@name Get and Set functions
+    ///@{
+
+    bool IsRational() const


Add Doxygen doc (same for everything)

loumalouomega

Commments, why ints?

loumalouomega · 2019-08-06T09:37:54Z

kratos/geometries/nurbs_curve_geometry.h

+        return mKnots;
+    }
+
+    int NumberOfKnots() const


Why int and not std::size_t?

The reason for the ints: The algorithms in the nurbs book assume that signed-integer operations can be performed. In some cases usigned integers led to wrong results. Since no error occurs, it is very difficult to debug something like this. Adapting the algorithms from the book is also not very easy in these cases.

loumalouomega · 2019-08-06T09:38:11Z

kratos/geometries/nurbs_curve_geometry.h

+
+    std::vector<Interval> KnotSpanIntervals() const
+    {
+        const int first_span = mPolynomialDegree - 1;


loumalouomega · 2019-08-06T09:39:16Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_curve_shape_functions.h

+#if !defined(KRATOS_NURBS_CURVE_SHAPE_FUNCTIONS_H_INCLUDED )
+#define  KRATOS_NURBS_CURVE_SHAPE_FUNCTIONS_H_INCLUDED
+
+#include "nurbs_utilities.h"


Separate properly:

// System includes // External includes // Project includes

loumalouomega · 2019-08-06T09:39:39Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_curve_shape_functions.h

+        mDerivativeOrder = DerivativeOrder;
+    }
+
+    int PolynomialDegree() const


Why int?, missing doxygen

loumalouomega · 2019-08-06T09:39:51Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_curve_shape_functions.h

+        return mPolynomialDegree;
+    }
+
+    int NumberOfNonzeroControlPoints() const


Idem in the following

loumalouomega · 2019-08-06T09:41:24Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_interval.h

+    ///@name Member Variables
+    ///@{
+
+    double m_t0;


kratos/geometries/nurbs_shape_function_utilities/nurbs_utilities.h

loumalouomega · 2019-08-06T09:41:55Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_utilities.h

+        const Vector& rKnots,
+        const double ParameterT)
+    {
+        auto span = std::lower_bound(std::begin(rKnots) + PolynomialDegree,


loumalouomega · 2019-08-06T09:42:03Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_utilities.h

+    ///@{
+
+    static int GetUpperSpan(
+        const int PolynomialDegree,


loumalouomega · 2019-08-06T09:42:27Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_utilities.h

+    {
+        // clang-format off
+        return
+            (K > N) ? 0 :  // out of range


I am not sure this is efficient

Actually, I thought so, too. However, precalculated values have produced surprisingly poor results. But it would still make sense to take the function from boost.

tteschemacher · 2019-08-06T10:17:51Z

Thanks @loumalouomega, I tried to change as suggested.

oberbichler · 2019-08-06T12:13:43Z

Thank you for the compliments but maybe there was a misunderstanding.

Last friday we decided that you would take care of the container for the shape functions. I should then adjust the geometries in my last pull-request as I discussed with @pooyan-dadvand and @RiccardoRossi in the bistro and use this container.

Why is there now a new pull-request containing only the curve geometry (without surfaces and tests) and without the container?

tteschemacher · 2019-08-06T14:45:43Z

I tried to ease the review process, further, I only adapted to the Kratos style.

oberbichler · 2019-08-06T15:27:55Z

This is basically a good idea but now we have pull-requests for the branches "nurbs-geometries" and "nurbs_geometries". Instead of creating this new branch (and making changes that lead to errors) it would make more sense to prepare the container as discussed last week.

tteschemacher · 2019-09-24T19:05:32Z

@philbucher you requested changes. I adressed your issues, are you ok with it now?

philbucher · 2019-09-25T10:52:20Z

can you please merge master?

philbucher

some more comments, but nothing significant

other than that for me is ok

kratos/geometries/nurbs_curve_geometry.h

philbucher · 2019-09-25T12:27:56Z

kratos/geometries/nurbs_curve_geometry.h

+            result[i] = Interval(t0, t1);
+        }
+
+        return result;


would using std::move make sense here?

not sure, asking the others

copy elision?

what do you mean?

std::move would make it more explicit (which is good) but as result is rvalue, the optimizer will use the move.

kratos/geometries/nurbs_curve_geometry.h

philbucher · 2019-09-25T12:35:28Z

kratos/geometries/nurbs_shape_function_utilities/nurbs_utilities.h

+        const Vector& rKnots,
+        const double ParameterT)
+    {
+        const auto span = std::lower_bound(std::begin(rKnots) + PolynomialDegree,


kratos/geometries/nurbs_shape_function_utilities/nurbs_utilities.h

kratos/tests/cpp_tests/geometries/test_nurbs_curve.cpp

philbucher · 2019-09-25T12:39:23Z

kratos/tests/cpp_tests/geometries/test_nurbs_curve_shape_functions.cpp

+        KRATOS_CHECK_NEAR(shape_functions(1, 6), -0.132366199073095, TOLERANCE);
+        KRATOS_CHECK_NEAR(shape_functions(2, 6), 0.161718941270974, TOLERANCE);
+        KRATOS_CHECK_NEAR(shape_functions(3, 6), -0.295903990157367, TOLERANCE);
+        KRATOS_CHECK_NEAR(shape_functions(4, 6), 0.227141701884515, TOLERANCE);


here you could use the new matrix-check macro

philbucher · 2019-09-25T12:40:27Z

kratos/tests/cpp_tests/geometries/test_nurbs_surface_shape_functions.cpp

+        KRATOS_CHECK_NEAR(shape_functions(8,14), 6.244572963750500e+00, TOLERANCE);
+        KRATOS_CHECK_NEAR(shape_functions(9,14), -4.328015755205562e+00, TOLERANCE);
+        KRATOS_CHECK_NEAR(shape_functions(10,14), -2.987503354121680e+00, TOLERANCE);
+        KRATOS_CHECK_NEAR(shape_functions(11,14), -1.031093322633297e+00, TOLERANCE);


same here

unrolling those macros apparently takes quite some time

…hape_functions.h Co-Authored-By: Philipp Bucher <philipp.bucher@tum.de>

…hysics/Kratos into core/nurbs_geometry

RiccardoRossi · 2019-09-26T20:17:30Z

this is the first one that should go in, right?

RiccardoRossi

as far as i can see this is essentially ready for merging.

@philbucher could u make a resume of what is still a TODO here?
please if you do tell what is crucial and what is "wish"

once that is done according to me this can go in

tteschemacher · 2019-09-27T08:38:39Z

Yes, this is very important

philbucher

to me looks ready
(although adding the docu as I pointed out would be nice ;) )

@pooyan-dadvand do you still want to take a look?

tteschemacher · 2019-09-27T10:41:24Z

I tried to adress all your comments. @pooyan-dadvand if you say yes I would merge.

pooyan-dadvand

👍

tteschemacher added 2 commits August 5, 2019 17:13

Curve geometry with shape function computation utilities.

4263906

Adding comments in geometry class

3c6b412

tteschemacher assigned oberbichler, RiccardoRossi, pooyan-dadvand, philbucher and loumalouomega Aug 6, 2019

tteschemacher added the Kratos Core label Aug 6, 2019

tteschemacher requested review from philbucher, pooyan-dadvand, RiccardoRossi, oberbichler and a team August 6, 2019 09:34

tteschemacher unassigned oberbichler, RiccardoRossi, pooyan-dadvand, loumalouomega and philbucher Aug 6, 2019

tteschemacher requested review from loumalouomega and andreas-apostolatos August 6, 2019 09:35

loumalouomega reviewed Aug 6, 2019

View reviewed changes

int -> size_t and some comments

cdad57d

some int -> size_t

5e19880

Small fixes

ee8ca68

Adding nurbs_curve test

e049f9a

tteschemacher added 2 commits September 24, 2019 18:20

updating geometry_dimension

5cf9003

geometry dimension fix

46e6ce0

Merge branch 'master' into core/nurbs_geometry

c3f697e

philbucher reviewed Sep 25, 2019

View reviewed changes

tteschemacher and others added 8 commits September 25, 2019 15:06

debug -> release error

d0f3e77

adding this

c1a8faa

remove unnecessary part

c387d68

Update kratos/geometries/nurbs_shape_function_utilities/nurbs_curve_s…

70bece1

…hape_functions.h Co-Authored-By: Philipp Bucher <philipp.bucher@tum.de>

class NurbsUtilities -> namespace

a304d14

Interval -> NurbsInterval

c7271b7

Merge branch 'core/nurbs_geometry' of https://github.com/KratosMultip…

97066a4

…hysics/Kratos into core/nurbs_geometry

neglect public

6601822

RiccardoRossi reviewed Sep 27, 2019

View reviewed changes

philbucher approved these changes Sep 27, 2019

View reviewed changes

philbucher added the Tobis Branch label Oct 1, 2019

pooyan-dadvand approved these changes Oct 1, 2019

View reviewed changes

tteschemacher merged commit 9adfe8c into master Oct 1, 2019

oberbichler deleted the core/nurbs_geometry branch December 12, 2019 14:31

philbucher mentioned this pull request Aug 26, 2020

Adding ModelPart KratosMultiphysics/CoSimIO#84

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core][NURBS] nurbs geometry #5361

[Core][NURBS] nurbs geometry #5361

tteschemacher commented Aug 6, 2019 •

edited

Loading

loumalouomega Aug 6, 2019

loumalouomega left a comment

loumalouomega Aug 6, 2019

oberbichler Aug 20, 2019

loumalouomega Aug 6, 2019

loumalouomega Aug 6, 2019

loumalouomega Aug 6, 2019

loumalouomega Aug 6, 2019

loumalouomega Aug 6, 2019

loumalouomega Aug 6, 2019

loumalouomega Aug 6, 2019

loumalouomega Aug 6, 2019

oberbichler Aug 20, 2019

tteschemacher commented Aug 6, 2019

oberbichler commented Aug 6, 2019

tteschemacher commented Aug 6, 2019

oberbichler commented Aug 6, 2019

tteschemacher commented Sep 24, 2019

philbucher commented Sep 25, 2019

philbucher left a comment

philbucher Sep 25, 2019

oberbichler Sep 27, 2019

philbucher Sep 27, 2019

pooyan-dadvand Sep 30, 2019

philbucher Sep 25, 2019

philbucher Sep 25, 2019

philbucher Sep 25, 2019

RiccardoRossi commented Sep 26, 2019

RiccardoRossi left a comment

tteschemacher commented Sep 27, 2019

philbucher left a comment

tteschemacher commented Sep 27, 2019

pooyan-dadvand left a comment

[Core][NURBS] nurbs geometry #5361

[Core][NURBS] nurbs geometry #5361

Conversation

tteschemacher commented Aug 6, 2019 • edited Loading

Choose a reason for hiding this comment

loumalouomega 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

tteschemacher commented Aug 6, 2019

oberbichler commented Aug 6, 2019

tteschemacher commented Aug 6, 2019

oberbichler commented Aug 6, 2019

tteschemacher commented Sep 24, 2019

philbucher commented Sep 25, 2019

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

RiccardoRossi commented Sep 26, 2019

RiccardoRossi left a comment

Choose a reason for hiding this comment

tteschemacher commented Sep 27, 2019

philbucher left a comment

Choose a reason for hiding this comment

tteschemacher commented Sep 27, 2019

pooyan-dadvand left a comment

Choose a reason for hiding this comment

tteschemacher commented Aug 6, 2019 •

edited

Loading