DLR-SC / tigl

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

Update generated classes #980

Closed MarAlder closed 7 months ago

MarAlder commented 7 months ago

This PR implements recent updates to CPACSGen. Essentially, this allows for cleaner querying of CPACS elements defined as simpleType embedded in complexType (e.g., wallSegment/phi).

Description

This PR is required because the commit ID (a1cadb8) of the linked git-submodule for CPACSGen and the existing cpacs_gen_input do not reproduce the generated C++ classes. This is because XSD restrictions minInclusive and maxInclusive were not yet implemented.

The idea behind this PR is therefore to update the git-submodule-link to the latest developments of CPACSGen, generate the corresponding classes and adapt TiGL accordingly. This should make it easier for developers to add classes for the implementation of new CPACS features.

1) Updated the CPACSGen submodule from a1cadb8 to 6f4fc1b 2) Generated the C++ classes based on the existing cpacs_gen_input definitions 3) Fixed custom classes to be consistent with the generated classes

How Has This Been Tested?

The impemented tests were carried out. One test fails (WingSegmentGuideCurves.tiglWingGetSegmentUpperSurfaceAreaTrimmed), but this is not related to the current changes.

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

N/A

Checklist:

joergbrech commented 7 months ago

Thanks @MarAlder! This is great.

One test fails (WingSegmentGuideCurves.tiglWingGetSegmentUpperSurfaceAreaTrimmed), but this is not related to the current changes.

But this test was successful before, so it must be related to changes here, right? Let's wait and see if it fails in CI.

codecov[bot] commented 7 months ago

Codecov Report

Merging #980 (dad59d4) into master (45a9428) will not change coverage. Report is 3 commits behind head on master. The diff coverage is 80.00%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/DLR-SC/tigl/pull/980/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/980?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 #980 +/- ## ======================================= Coverage 68.90% 68.90% ======================================= Files 298 298 Lines 26479 26479 ======================================= Hits 18246 18246 Misses 8233 8233 ``` | [Files](https://app.codecov.io/gh/DLR-SC/tigl/pull/980?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC) | Coverage Δ | | |---|---|---| | [src/CCPACSStringVector.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/980?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL0NDUEFDU1N0cmluZ1ZlY3Rvci5jcHA=) | `86.79% <100.00%> (ø)` | | | [src/cpacs\_other/CCPACSExternalObject.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/980?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2NwYWNzX290aGVyL0NDUEFDU0V4dGVybmFsT2JqZWN0LmNwcA==) | `55.07% <100.00%> (ø)` | | | [.../structural\_elements/CCPACSFuselageWallSegment.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/980?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL3N0cnVjdHVyYWxfZWxlbWVudHMvQ0NQQUNTRnVzZWxhZ2VXYWxsU2VnbWVudC5jcHA=) | `75.83% <50.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/DLR-SC/tigl/pull/980/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC)
MarAlder commented 7 months ago

Yep, the test also fails on my local copy of master. Seems to be something with my local settings. I'll check that.

True, the python bindings need to be adjusted as well.

rainman110 commented 7 months ago

@MarAlder I could be simply a different OCCT version, leading to slight changes in the area computation. Another possibility is, that you don't have a patched OCCT for C2 continous coons surfaces. This will also lead to different shapes and areas.

joergbrech commented 7 months ago

I just had a look. Fixing the python bindings boils down to removing four lines from bindings/python_internal/configuration.i:

diff --git a/bindings/python_internal/configuration.i b/bindings/python_internal/configuration.i
index a436d12a..d7049d34 100644
--- a/bindings/python_internal/configuration.i
+++ b/bindings/python_internal/configuration.i
@@ -90,7 +90,6 @@
 #include "generated/CPACSWallPositionUIDs.h"
 #include "generated/CPACSWallPosition.h"
 #include "generated/CPACSWallPositions.h"
-#include "generated/CPACSWallSegment_phi.h"
 #include "generated/CPACSWallSegment.h"
 #include "generated/CPACSWallSegments.h"
 #include "generated/CPACSUIDSequence.h"
@@ -130,7 +129,6 @@
 %boost_optional(tigl::CCPACSWingRibsDefinitions)
 %boost_optional(tigl::CCPACSWingSpars)
 %boost_optional(tigl::generated::CPACSGuideCurve_continuity)
-%boost_optional(tigl::CCPACSRectangleProfile_cornerRadius)
 %boost_optional(tigl::CCPACSRectangleProfile)
 %boost_optional(tigl::CCPACSStandardProfile)
 %boost_optional(tigl::CCPACSWingProfileCST)
@@ -187,7 +185,6 @@
 %include "generated/CPACSWallPositionUIDs.h"
 %include "generated/CPACSWallPosition.h"
 %include "generated/CPACSWallPositions.h"
-%include "generated/CPACSWallSegment_phi.h"
 %include "generated/CPACSWallSegment.h"
 %include "generated/CPACSWallSegments.h"
 %boost_optional(tigl::generated::CPACSWalls)
@@ -405,7 +402,6 @@ class CCPACSWingRibsPositioning;
 %include "generated/CPACSCst2D.h"
 %include "ITiglWingProfileAlgo.h"
 %include "generated/CPACSPosExcl0DoubleBase.h"
-%include "generated/CPACSRectangleProfile_cornerRadius.h"
 %include "generated/CPACSRectangleProfile.h"
 %include "generated/CPACSStandardProfile.h" //TODO: Need to replace with implementation CCPACSStandardProfile.h, once it exists.
 %include "CCPACSWingProfileCST.h"

CPACSWallSegment::phi and CPACSRectangleProfile::cornerRadius are now double values and don't have an extra wrapping type.

@MarAlder, do you want to apply these changes to this PR or do you want me to take over from here?

MarAlder commented 7 months ago

I just had a look. Fixing the python bindings boils down to removing four lines from bindings/python_internal/configuration.i: [...] @MarAlder, do you want to apply these changes to this PR or do you want me to take over from here?

Ok, I suspected that too, but have not yet been able to verify it. I'll have to check again locally that I can compile the internal bindings correctly, but that's a different issue that I might get back to you with. Thanks for testing, if you could directly apply the changes that's fine for me, then we can finalize the PR.

joergbrech commented 7 months ago

Looks good! Thanks again!