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

970 make tiglwinginterpolatexsi work on a single segment that is not part of a component segment #979

Conversation

AntonReiswich
Copy link
Contributor

@AntonReiswich AntonReiswich commented Dec 6, 2023

Description

In order to fix #970, the function tiglWingInterpolateXsi has been generalized. The generalization allows to not only use the function for segments that are not contained in a component segment (c.f. #970), but also more generally. Consider the following illustration that demonstrates the generality in which tiglWingInterpolateXsi can be used now:

segments and css_neu

Assume the depicted wing has four segments S1, S2, S3 and S4 and two component segments CS_A and CS_B.
The three arguments of the function firstUID, secondUID, intersectionUID can be now chosen (to give only few exapmles) in the following way:

  • S3, S3, S3 (which fixes Make tiglWingInterpolateXsi work on a single segment, that is not part of a component segment  #970)
  • S1, S3, CS_A
  • S1, S3, S3
  • CS_A, CS_B, S1
  • CS_A, CS_B, S3
  • ...
    More precisely: firstUID, secondUID, intersectionUID can be either a segment or component segment, respectively. The segments don't necessarily need to belong to a component segment. The component segments refered by the UIDs don't need to be the same. The only restriction is that all segments or component segments refered by the three UIDs are part of one and the same wing. If the intersection if outside the segment or component segment refered by intersectionUID an error gets thrown, as for example if one chooses S3, S4, S1.

How Has This Been Tested?

New test data (test_wing_segment_special_modified_component_segments.xml) has been added together with unittests in WingInterpolateXsi to check new functionalities that on the one hand show that the bug in issue #970 is fixed and on the other hand to check the functionalities that came with generalizing the function even further.

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.

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (bd9dca3) 68.75% compared to head (b9db981) 68.94%.
Report is 27 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #979      +/-   ##
==========================================
+ Coverage   68.75%   68.94%   +0.18%     
==========================================
  Files         299      299              
  Lines       26481    26509      +28     
==========================================
+ Hits        18208    18277      +69     
+ Misses       8273     8232      -41     
Files Coverage Δ
src/wing/CCPACSWingComponentSegment.cpp 76.28% <100.00%> (+1.33%) ⬆️
src/wing/tigletaxsifunctions.cpp 81.45% <96.36%> (-6.05%) ⬇️

... and 9 files with indirect coverage changes

@AntonReiswich AntonReiswich marked this pull request as draft December 6, 2023 13:49
@AntonReiswich AntonReiswich marked this pull request as ready for review December 21, 2023 17:29
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 @AntonReiswich. I just have some minor review comments.

tests/unittests/testWingInterpolateXsi.cpp Outdated Show resolved Hide resolved
tests/unittests/testWingInterpolateXsi.cpp Outdated Show resolved Hide resolved
tests/unittests/testWingInterpolateXsi.cpp Outdated Show resolved Hide resolved
tests/unittests/testWingInterpolateXsi.cpp Outdated Show resolved Hide resolved
tests/unittests/testWingInterpolateXsi.cpp Outdated Show resolved Hide resolved
src/wing/tigletaxsifunctions.cpp Outdated Show resolved Hide resolved
src/wing/tigletaxsifunctions.cpp Outdated Show resolved Hide resolved
src/wing/tigletaxsifunctions.cpp Outdated Show resolved Hide resolved
src/wing/tigletaxsifunctions.cpp Outdated Show resolved Hide resolved
src/wing/tigletaxsifunctions.cpp Outdated Show resolved Hide resolved
@AntonReiswich
Copy link
Contributor Author

Thanks a lot for the code review!

@joergbrech joergbrech merged commit 8918ca3 into master Jan 3, 2024
17 checks passed
@joergbrech joergbrech deleted the 970-make-tiglwinginterpolatexsi-work-on-a-single-segment-that-is-not-part-of-a-component-segment branch January 3, 2024 13:25
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.

Make tiglWingInterpolateXsi work on a single segment, that is not part of a component segment
2 participants