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

additional SBM features to nurbs_curve_on_surface_geometry #12717

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

NickNick9
Copy link
Contributor

📝 Description
We have added to the nurbs_curve_on_surface_geometry the possibility of considering an SBM brep surface.
Instead of using the method ComputeAxisIntersection, in the SBM case we use a different one called ComputeAxisIntersectionSBM (that only looks for intersections on the knot vertices).

Moreover, we have also added the SBM case in the CreareQuadraturePoints method in the same file.

IgaApplication
Geometry

Co-authored-by: andrewgorgi <[email protected]>
const double tolerance_intersection = 1e-15;

// Scale factor between volume coordinate and surface one
double physical_length_segment = norm_2(physical_coord_1-physical_coord_2);
Copy link
Member

Choose a reason for hiding this comment

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

const?

kratos/sources/variables.cpp Outdated Show resolved Hide resolved
Copy link
Member

@rubenzorrilla rubenzorrilla left a comment

Choose a reason for hiding this comment

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

I need to do a further review, but at first glance, I'm not a big fan of adding a geometry that depends on the method... In other words, I don't really like the IS_SBM stuff. Can you please justify its need?

@NickNick9
Copy link
Contributor Author

@rubenzorrilla > I need to do a further review, but at first glance, I'm not a big fan of adding a geometry that depends on the method... In other words, I don't really like the IS_SBM stuff. Can you please justify its need?

@NickNick9
The fact is that our SBM stuff is very similar to the IBRA one. So we basically use the same geometries, but at the very end, we only cut along the knot vertices. So in more than one place, we have to differentiate between the two (Create an additional function SpansLocalSpace_SBM would be complex because it is called in many other places and still we would need a flag I guess). We are planning to use the IS_SBM when we create the gauss points for example.

So my reason is: SBM and IBRA conceptually are very similar, we want a flag that says which approach you want to use when you have to deal with the un-fitted boundary in IGA.

Copy link
Member

@rubenzorrilla rubenzorrilla left a comment

Choose a reason for hiding this comment

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

Overall makes sense. Nevertheless, I'd indicate the upcoming changes with code comments. Specially the unification of the SBM and the untrimmed cases, which is something that IMO must be done for sure (most probably in a future PR).

@rubenzorrilla
Copy link
Member

Overall makes sense. Nevertheless, I'd indicate the upcoming changes with code comments. Specially the unification of the SBM and the untrimmed cases, which is something that IMO must be done for sure (most probably in a future PR).

Did you already add the comments I mentioned in here? (I did not find them).

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.

4 participants