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

Feature: IfcSweptDiskSolidPolygonal Informal Proposition #3

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

aothms
Copy link
Collaborator

@aothms aothms commented Sep 29, 2022

This PR adds support for the Informal Proposition that compares FilletRadius to the length of the segments of the directrix.

@aothms
Copy link
Collaborator Author

aothms commented Oct 5, 2022

Sandbox ok a1d0aa7

Copy link
Contributor

@pjanck pjanck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First comments. Did not look through the whole PR yet.

Comment on lines +10 to +11
Then FilletRadius has to be smaller than or equal to the length of the start and end segments of the Directrix
And FilletRadius has to be smaller than or equal to the length / 2 of the inner segments of the Directrix
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breadcrumbs: #20 - here is another section that should be amended accordingly if we strive for maximum reusability.

And FilletRadius = not null
And Directrix forms a closed curve

Then FilletRadius has to be smaller than or equal to the length of all segments of the Directrix
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come not length / 2, similarly to the open curve example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pjanck I think you're correct. A closed curve means all segments are the "inner segments" and hence the length / 2 rule should apply. Thank you.

There are also no test cases yet for closed curve directrices.

And FilletRadius = not null
And Directrix forms an open curve

Then FilletRadius has to be smaller than or equal to the length of the start and end segments of the Directrix
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change "shall" to "must" as per #20

And FilletRadius = not null
And Directrix forms a closed curve

Then FilletRadius has to be smaller than or equal to the length of all segments of the Directrix
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem, change "shall" to "must" as per #20

@aothms aothms marked this pull request as draft February 1, 2023 08:07
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.

None yet

3 participants