-
Notifications
You must be signed in to change notification settings - Fork 74
Approximate B-Spline Profile Point Lists by defining a Control Point Number #1277
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
Open
svengoldberg
wants to merge
15
commits into
main
Choose a base branch
from
approxBSplineProfilePointList
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hout setter. Also, later no casting needed
Additionally added options: User can define the approximation error method, currently: root mean square error or maximum error. Furthermore, the user can pass an index list within the CPACS configuration to define which points should still be interpolated. The other points, however, are approximated. The unittests needed to be changed to test for this behaviour. Also, a mistake was fixed, since the was a confusion regarding 0-based and 1-based indexing within th logic of interpolation indices.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1277 +/- ##
==========================================
- Coverage 72.46% 72.19% -0.27%
==========================================
Files 313 314 +1
Lines 27395 27592 +197
==========================================
+ Hits 19851 19920 +69
- Misses 7544 7672 +128
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
General
This PR introduces the possibility to create b-spline profiles by approximating the user-set CPACS point list.
This is implemented for fuselage profiles and wing airfoils currently. It is still possible to define an index list of points that should be interpolated.
Thanks to @sdeinert for coming up with the CPACS definition!!
We extended it by the option to pass the error computation method and the optional index list for points that should be interpolated.
@MarAlder what do you think about our suggestion to extend the CPACS schema?
Code Behaviour
The user can now define the wanted number of control points that is used to approximate the profile. When kinks or guide curves are defined, this number may still be larger than what is defined in CPACS, for mathematical reasons. Nevertheless, the user now has more flexibility to determine the geometric behaviour of each profile. As a natural result from this reduced number of control points, an approximation error is introduced. The user can currently select from the computation as the root mean square error or the maximum error within CPACS. The respective choice and value are printed within the
TiGLCreatorGUI for better feedback.In CPACS we also defined the other way round: Meaning, that a maximum tolerance for the approximation error can be defined and TiGL defines the control point number on its own. But, this still needs to be implemented in another pull request.
Unittests are added for both a wing and fuselage profile.
Implementation Details and Code Refactorings
The present implementation comes with quite a few changes and refactorings of yet existing code.
WireCachetype defined insrc/wing/CTiglWingProfilePointList.his split up. Before, it contained both the opened and closed wire. The save computation time and output, the cache is split into two different ones. Only the needed one (closed or opened) is computed. For that, parts of the class had the be rewritten.src/geometry/CTiglApproximateBsplineWirewas refactored to account for the new CPACS nodes and for it to store the necessary variables.src/geometry/CTiglBSplineApproxInterpand its already defined functions which is almost unused up to now. Here, the error computation is changed to be more accurate. This is also the explanation for the needed change of the unittest value. Now, the approximation error is computed based on the difference between the beforehand defined points in the point list and an (approximation) of their projection on the new b-spline curve. Also, the error actual computation (2 point vectors) is outsourced to functions insrc/common/tiglcommonfunctions. The function is passed as a function pointer to thesolvemethod for more flexibility as we allow different ways of computation.src/geometry/CTiglApproximateBsplineWireis now also based on CTiglBSplineApproxInterp to make use of existing code and to be consistent with the fuselage implementation. Before, an OCCT-function was called here. However, we wanted to define the number of control points which was not really possible with the OCCT-function.computeParamsfunctions fromsrc/geometry/CTiglInterpolatePointsWithKinkshas to move from an anonymous namespace into thetiglnamespace since it is also needed outside from this class.Fixes #1276.
Checklist: