DLR-SC / tigl

The TiGL Geometry Library to process aircraft geometries in pre-design.
https://dlr-sc.github.io/tigl/
Apache License 2.0
235 stars 60 forks source link

Made Lofting algorithm throw Exception when side caps are invalid #999

Closed merakulix closed 1 month ago

merakulix commented 7 months ago

Description

Throwing an exception while building the shells, using invalid side caps, prevents TiGL to build invalid solid shapes.

How Has This Been Tested?

Added test in testFuselageStandardProfileRectangle.cpp that checks for exception if side caps are invalid. If rectangular profile wires are built that cannot be used to create valid side caps, the exception is thrown.

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

This solid (created by a rectangle profile with too large corner radius) is not valid. Building side caps from its closed, but entangled profile wire is not possible. Solids like this will not be built and used mistakenly, if exception is thrown.

grafik

Checklist:

joergbrech commented 5 months ago

As per our chat on MM: Let's make sure that this doesn't affect the TiGL performance too much. One option could be to activate the test only in debug mode (and optionally: If we have the check in debug mode only, we could use simple assertions rather than exceptions. I believe this is common for checks that are only performed in debug mode)

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.02%. Comparing base (a334bd3) to head (6c4fd02). Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
src/geometry/CTiglPatchShell.cpp 50.00% 2 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/DLR-SC/tigl/pull/999/graphs/tree.svg?width=650&height=150&src=pr&token=8b0i0TsOw6&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC)](https://app.codecov.io/gh/DLR-SC/tigl/pull/999?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC) ```diff @@ Coverage Diff @@ ## master #999 +/- ## ========================================== - Coverage 70.03% 70.02% -0.01% ========================================== Files 301 301 Lines 24312 24316 +4 ========================================== + Hits 17026 17028 +2 - Misses 7286 7288 +2 ``` | [Flag](https://app.codecov.io/gh/DLR-SC/tigl/pull/999/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/DLR-SC/tigl/pull/999/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC) | `70.02% <50.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/DLR-SC/tigl/pull/999?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC) | Coverage Δ | | |---|---|---| | [src/logging/CTiglLogging.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/999?src=pr&el=tree&filepath=src%2Flogging%2FCTiglLogging.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2xvZ2dpbmcvQ1RpZ2xMb2dnaW5nLmNwcA==) | `80.39% <ø> (ø)` | | | [src/geometry/CTiglPatchShell.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/999?src=pr&el=tree&filepath=src%2Fgeometry%2FCTiglPatchShell.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2dlb21ldHJ5L0NUaWdsUGF0Y2hTaGVsbC5jcHA=) | `91.25% <50.00%> (-2.18%)` | :arrow_down: |
joergbrech commented 1 month ago

I am sorry, could you please have another look at the coverage report?

joergbrech commented 1 month ago

I am sorry, could you please have another look at the coverage report?

NVM, the uncovered lines are in a fallback implementation for non-planar profiles, which are not supported by TiGL anyway. Constructing a unit test for this exotic case just for the sake of code coverage seems like too much work. Let's merge