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

Adding sections with guide curves not working #626

Closed
rainman110 opened this issue Sep 26, 2019 · 3 comments
Closed

Adding sections with guide curves not working #626

rainman110 opened this issue Sep 26, 2019 · 3 comments

Comments

@rainman110
Copy link
Collaborator

rainman110 commented Sep 26, 2019

This applies for fuselages and wings.

TiGL should not crash or do unpredictable things when adding sections in a segment with existing guide curves. At the very least, TiGL should raise an error. --> Minimum requirement before merging to master.

If a section is created before or after a segment with guide curves, we could in theory add a dummy guide curve that connects to the guide curves of the segment.

Conceptually, guide curves are defined dependent of the sections. So a user should ideally create all sections first and then add guide curves.

svengoldberg added a commit that referenced this issue Nov 26, 2024
The CPACS creator currently allows adding sections in segments (or at their boundaries) when they contain guide curves. However this leads to crashes or undefined behaviour. As it might be hard to discuss reasonable behaviour or predict what the user wants in complex examples, we do not allow this right now.
(At least partly) fixes issue #626
@svengoldberg
Copy link
Contributor

@joergbrech @merakulix @AntonReiswich We discussed this in the last session. IIRC, our idea was to throw an exception when sections are added in segments with gc. We also agreed on leaving it like this and see whether users are really seeking for this feature.
In this case we could think about implementing way adding those sections. Nevertheless, there are open questions on how exactly this should behave.

If you are fine with my suggested solution (so just throw an error and do not allow it; see 8f1c05a), I can open the PR.

@joergbrech
Copy link
Contributor

A quick code-review on your commit here: Lets be specific in the error message that it is not supported out of principle, but it is not supported for now. We could even include the information i the error message that guide curves should be added once all sections are defined, which IMHO makes sense, because that's basically the data dependency in CPACS (guide curves reference existing sections, not vice versa). What do you think?

@svengoldberg
Copy link
Contributor

svengoldberg commented Nov 26, 2024

Yes of course, that sounds reasonable.
The hint regarding the data dependency is also a good idea. I will add this

https://github.com/DLR-SC/tigl/commits/626_cpacs_creator_add_section_gc/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants