Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add functionality for reparameterizing B-splines without changing geometry (Fix issue #890) #1007

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

svengoldberg
Copy link
Contributor

@svengoldberg svengoldberg commented May 7, 2024

Description

This PR implements a method to apply exact reparameterizations of B-splines used for profile curves. The underlying algorithm was found in The NURBS Book (1997; 2nd edtion; Piegl and Tiller), p. 251. The reparameterization is mandatory for lofting (correct connection of kinks in two profiles, since they are connected based on parametric values of the curves).
The current implemented reparameterization creates overshoots of the profile curves (cf. issue #890) due to change of the geometry. The approach in this PR does not change the geometric structure but still executes a reparameterization.
The wanted parameters are interpolated picewise linearly (to mimic a B-spline structure of the reparameterization function) and to not increase the degree of the resulting B-spline curve. After that, the above mentiond algorithm is applied based on this reparameterization function.
To come into account, a new function and some helper functions are implemented based on OpenCASCADE B-spline handling.

Changes in src/fuselage/CCPACSFuselageProfile.cpp :

Apply the reparameterization on a B-spline. Similar to the current behaviour.

Changes in src/geometry/CTiglBSplineAlgorithms.* :

Implement the new reparameterization (bsplineReparameterizePicewiseLinear) with the use of a few helper functions.

Changes in src/geometry/CTiglInterpolatePointsWithKinks.* :

The function computeParams is now needed also in CCPACSFuselageProfile.cpp and is therefore put into a non-anonymous namespace.

Fixes issue #890

EDITS:

  • The combination of this reparameterization with the reparametrizeBSplineNiceKnots() seems not to work as the geometry is changed in a too drastic way when the parameters are predefined. However, since the performance advantage should not be rejected in the most cases, the function is only used when there are no parameters defined in the CPACS profile.
  • The tolerance is not only applied on the knot removal but also on the knot refinement (-> insertion) and following steps
  • Due to these changes, the PR fixes issue Fuselage reparametrization leads to crash on model with kinks #770

How Has This Been Tested?

The test bsplineReparameterizePicewiseLinear is added to check if the reparameterization is applied correct and if the geometric structure is kept. Also, in the attached screenshots, the behaviour of the old vs. new implementation is compared:

Overshoot
Overshoot after reparameterization
Fix_Overshoot
No overshoot after reparameterization

Checklist:

  • [x ] A test for the new functionality was added.
  • All tests run without failure.
  • The new code complies with the TiGL style guide.
  • New classes have been added to the Python interface.
  • API changes were documented properly in tigl.h.

@rainman110
Copy link
Collaborator

rainman110 commented May 8, 2024

@svengoldberg Great effort! I saw that you chose a piecewise linear approach. Doesn't it introduce c1-discontinuities (i.e. jumps in the curve's gradient) at the edges of each curve segment? If so, this could be a problem for the lofting algorithm as it can introduce g1-discontinuities during lofting (i.e. undesired kinks).

We had the same idea back then with Merlin (who implemented the Gordon Surface Algorithm), where we first decided to implement a piecewise linear reparametrization and experienced these problems (kinks) in some of the gordon surface test cases.

As a consequence, we chose the continuous approximative reparametrization, which keeps the degree and a smooth reparametrization curve - however with the overshooting problems you discovered.

@rainman110
Copy link
Collaborator

I think for the case to introduce kinks into the surface, your method is correct. But I recommend also looking at cases where the curve is reparametrized but without introducing any kinks.

@svengoldberg svengoldberg reopened this May 8, 2024
@joergbrech
Copy link
Contributor

I think for the case to introduce kinks into the surface, your method is correct. But I recommend also looking at cases where the curve is reparametrized but without introducing any kinks.

Good point! Let's check what happens if we have two curves without kinks, reparametrize at least one of them piecewisely linear and create a loft.

@rainman110 I didn't know you tried this with Merlin back then, good to know and thanks for the heads up! Anyway, with @svengoldberg's changes we now have two algorithms, the approximate one from before and the piecewise linear one, both with their pros and cons

@rainman110
Copy link
Collaborator

Anyway, with @svengoldberg's changes we now have two algorithms, the approximate one from before and the piecewise linear one, both with their pros and cons

Yes, I think it is a great addition. Ideally, the algorithm is chosen "dynamically" depending on what is needed.

@svengoldberg
Copy link
Contributor Author

svengoldberg commented May 8, 2024

Yes, I think it is a great addition. Ideally, the algorithm is chosen "dynamically" depending on what is needed.

@joergbrech already had an idea to restructure the additions and I implemented it locally. I will push the changes later (after some more testing)

@svengoldberg
Copy link
Contributor Author

@rainman110 I took the same CPACS file as at the top just left out all kinks in the profile definition. It looks like this should not be a problem:

reparam_wo_kinks_1
reparam_wo_kinks_2

@rainman110
Copy link
Collaborator

@rainman110 I took the same CPACS file as at the top just left out all kinks in the profile definition. It looks like this should not be a problem:

Have a look with the zebra plot and a high tesselation. These problems are not easy to catch visually, but can be indeed an aerodynamic problem. The zebra stripes need to pass all "edges" in a smooth way

@svengoldberg
Copy link
Contributor Author

@rainman110 I did some testing and changed the parameters in a way that I took different number of parameters on two lofted profiles and raised the "veloctity" of the curve between two points. I could not reproduce this problem... To me, it looked like, the zebra stripes do pass in a smooth way. Do you maybe still have an idea about a configuration that you tested which created this problem of missing C1-continuity?

@rainman110
Copy link
Collaborator

rainman110 commented May 13, 2024

Have a look at the gordon surface case test_surface_4: (https://github.com/DLR-SC/tigl/tree/master/tests/unittests/TestData/CurveNetworks/test_surface4)

Here, profiles should be reparametrized, such that all intersections are at the same curve parameter. Then create a skinned surface.

Ideally, the skinned surface is

  • C1 Smooth
  • and still interpolates the input curves

Alternatively, replace your new reparametrization algorithm with the one used by the gordon surface method:

The resulting surface needs to interpolate the input curves. Back then, they did not exactly (it was not easy too see though).

This should give as a good feeling, for which algorithm your new reparametrization function can be used (and for what not).

When there are parameters defined in the CPACS profile, the reparameterization in CTiglInterpolatePointsWithKinks() does not get along with the following reparametrizeBSplineNiceKnots() which was implemented for performance reasons. In this case, the nice knots change the geometry in a too drastic way (cf. unittest BSplineInterpolation.reparameterizePiecewiseLinear). To still account for these performance advantages in most cases, the function is reactivated. But only if there are no parameters defined in the CPACS profile.
@svengoldberg
Copy link
Contributor Author

@rainman110 We investigated the application of the new algorithm within the Gordon surface method. However, the guide curve's poles of the resulting object looked not very nice (not equidistant, stretched/clinched in 'guide curve direction') and we could not really discuss the continuity property. In addtion to that, 3 other Gordon surface tests did not pass when both (profiles and guides) are reparameterized using the new algorithm. Nevertheless, the test you mentioned passes. When only the profiles are reparameterized this way, everything works fine and all tests pass.
So all in all, it seems to be best not to change the way the curves are reparameterized inside the Gordon surface method and only keep this new algorithm for the fuselages.

@rainman110
Copy link
Collaborator

@svengoldberg Thanks for the update

Copy link
Contributor

@joergbrech joergbrech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @svengoldberg! I have a few comments, mostly cosmetics.

src/geometry/CTiglBSplineAlgorithms.h Outdated Show resolved Hide resolved
src/geometry/CTiglInterpolatePointsWithKinks.cpp Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.cpp Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.cpp Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.cpp Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.cpp Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.cpp Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.h Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.cpp Outdated Show resolved Hide resolved
src/geometry/CTiglBSplineAlgorithms.cpp Outdated Show resolved Hide resolved
Tolerance was only implemented in knot removal before, but now for compatability also added for knot insertion. There existed CPACS files that crashed without this tolerance for adding knots in the first step of the reparameterization algorithm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants