Resolve: Enable to use more than 3 wing guide curves #748

rainman110 · 2020-10-09T15:34:15Z

This PR enables to use more than 3 guide curves with TiGL.

Description

Several issues had been addressed:

Correctly detect upper and lower wing surfaces
Added algorithm to concate surfaces
Concat all upper / lower surfaces of a wing segment, that is used for the get point methods and others as well.

How Has This Been Tested?

TODO: We still need a test example.xml file, that demonstrates the functionality. We might use the FFD or another aircraft.

Screenshots, that help to understand the changes(if applicable):

Checklist:

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.

codecov-io · 2020-10-09T19:02:06Z

Codecov Report

Merging #748 into master will increase coverage by 0.41%.
The diff coverage is 96.59%.

@@            Coverage Diff             @@
##           master     #748      +/-   ##
==========================================
+ Coverage   69.00%   69.42%   +0.41%     
==========================================
  Files         423      425       +2     
  Lines       23177    23339     +162     
==========================================
+ Hits        15994    16202     +208     
+ Misses       7183     7137      -46

Impacted Files	Coverage Δ
src/geometry/CTiglBSplineAlgorithms.h	`100.00% <ø> (ø)`
src/wing/CTiglWingBuilder.cpp	`90.90% <89.18%> (+23.74%)`	⬆️
src/geometry/CTiglConcatSurfaces.cpp	`92.85% <92.85%> (ø)`
src/wing/CCPACSWing.cpp	`80.43% <95.00%> (+0.51%)`	⬆️
src/wing/CCPACSWingSegment.cpp	`81.13% <97.05%> (+0.51%)`	⬆️
src/common/tiglcommonfunctions.h	`100.00% <100.00%> (ø)`
src/geometry/CTiglBSplineAlgorithms.cpp	`96.49% <100.00%> (+0.99%)`	⬆️
src/geometry/CTiglConcatSurfaces.h	`100.00% <100.00%> (ø)`
src/geometry/CTiglGordonSurfaceBuilder.cpp	`94.00% <100.00%> (ø)`
src/geometry/CTiglTransformation.cpp	`79.04% <100.00%> (+0.63%)`	⬆️
... and 11 more

src/wing/CCPACSWing.cpp

rainman110 · 2020-10-13T14:44:24Z

@joergbrech I know this PR is plenty of code, but could you have a look to it?

joergbrech

Nice! I still have a few small comments. What exactly is the purpose of approximating the input surfaces for the concatenation?

src/geometry/CTiglBSplineAlgorithms.cpp

src/geometry/CTiglBSplineAlgorithms.h

src/geometry/CTiglConcatSurfaces.h

src/geometry/CTiglTransformation.h

src/wing/CTiglWingBuilder.cpp

src/guide_curves/CCPACSGuideCurves.cpp

Addresses #687

I also changed the tolerances slightly due to numerical issues.

The current implementation is a bit hacky, as we currently have to create a face to transform and then extract again the surface.

This commit concatenates the surfaces of a segment to provide a single surface to make the GetPoint functions work. Addresses #687

This checks, whether the mechanism is really independent of the order of guides.

This tests case check the get point functions on the guide curve case with four guide curves.

We had plenty of code disabled by the preprocessor flag NO_EXPLICIT_TE_MODELING. This code was not tested anymore and is probably buggy.

I also added a better doc string and improved the docstring of CTiglConcatSurfaces

rainman110 · 2020-10-15T13:08:09Z

@joergbrech All actions should be addressed now.

I rebased finally onto master to be able to merge.

joergbrech · 2020-10-15T13:44:01Z

Awesome! LGTM

rainman110 commented Oct 12, 2020

View reviewed changes

src/wing/CCPACSWing.cpp Show resolved Hide resolved

rainman110 marked this pull request as ready for review October 12, 2020 20:49

joergbrech self-requested a review October 13, 2020 15:09

joergbrech requested changes Oct 14, 2020

View reviewed changes

rainman110 commented Oct 14, 2020

View reviewed changes

src/guide_curves/CCPACSGuideCurves.cpp Show resolved Hide resolved

rainman110 mentioned this pull request Oct 15, 2020

Remove reparametrizeBSpline from createCommonKnotsVectorCurve #751

Open

rainman110 added 22 commits October 15, 2020 14:39

Correctly map wing surface names in case of more than 3 guide curves

97317ee

Addresses #687

Fixed compilation with g++

6d3c4e1

Addresses #687

Removed log message

7a7cf22

Addresses #687

Fixed face names of wing segment

3b7ff53

fixed bug in TiGL Viewer

48dcdc0

Added functions to match knots of surfaces

c6ec8d5

Added some geometry functions to python bindings

dd1486c

Added function to change parametrization of surfaces

ed8a4e3

I also changed the tolerances slightly due to numerical issues.

Added surface concatenation algorithm

99a9c6b

Added tests for concatCurvesU

e21a6be

Added very simple surface approximation algorithm

42330c9

Added test for surface approximation

6d3c74c

Fixed a bug

814347a

Added algorithm to concat multiple surfaces

f0bd217

Extended CTiglTransformation to transform surfaces

ab2974d

The current implementation is a bit hacky, as we currently have to create a face to transform and then extract again the surface.

Set cpp11 requirement for tigl_3cpp target

b4a0ee9

Concatenate surfaces to make getpoint working

28e3709

This commit concatenates the surfaces of a segment to provide a single surface to make the GetPoint functions work. Addresses #687

Fixed bug

30ead36

Added cpacs test file

cabb148

Some code improvements

6d8491d

Shuffled guide curves

9aaebd1

This checks, whether the mechanism is really independent of the order of guides.

Added test case for surface transformation

0989e7e

rainman110 added 14 commits October 15, 2020 14:41

Some simplification

7a090aa

Added one more test case

76fb237

This tests case check the get point functions on the guide curve case with four guide curves.

Added tests for concatsurface class

d11160b

Moved unused code into ifdef

42e810c

Removed dead code

d799916

We had plenty of code disabled by the preprocessor flag NO_EXPLICIT_TE_MODELING. This code was not tested anymore and is probably buggy.

Moved FindIndex and Contains to tiglcommonfunctions.h

4c0b203

Added tests and documentation for new functions

650c689

Fixed copy & paste error

70ddea2

Renamed function approxSurface to makeKnotsUniform

e73737f

I also added a better doc string and improved the docstring of CTiglConcatSurfaces

Removed duplicate code in createCommonKnotsVectorSurface

936699a

Avoid code duplication of guide curve start parameters

61a00c1

Fixed code according to comments

845036f

Changed code to comply with tolerance at other places

57bdbf3

Fixed warnings

fcd82a3

rainman110 force-pushed the 687_fix_wing_guide_curves branch from 1d31d1e to fcd82a3 Compare October 15, 2020 13:06

joergbrech approved these changes Oct 15, 2020

View reviewed changes

joergbrech merged commit b0ca5e6 into master Oct 15, 2020

rainman110 deleted the 687_fix_wing_guide_curves branch October 15, 2020 14:23

rainman110 added this to In progress in TiGL 3.2 via automation Oct 19, 2020

rainman110 moved this from In progress to Done in TiGL 3.2 Oct 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve: Enable to use more than 3 wing guide curves #748

Resolve: Enable to use more than 3 wing guide curves #748

rainman110 commented Oct 9, 2020 •

edited

codecov-io commented Oct 9, 2020 •

edited

rainman110 commented Oct 13, 2020

joergbrech left a comment

rainman110 commented Oct 15, 2020

joergbrech commented Oct 15, 2020

Resolve: Enable to use more than 3 wing guide curves #748

Resolve: Enable to use more than 3 wing guide curves #748

Conversation

rainman110 commented Oct 9, 2020 • edited

Description

How Has This Been Tested?

Screenshots, that help to understand the changes(if applicable):

Checklist:

codecov-io commented Oct 9, 2020 • edited

Codecov Report

rainman110 commented Oct 13, 2020

joergbrech left a comment

Choose a reason for hiding this comment

rainman110 commented Oct 15, 2020

joergbrech commented Oct 15, 2020

rainman110 commented Oct 9, 2020 •

edited

codecov-io commented Oct 9, 2020 •

edited