Skip to content

Conversation

@simlapointe
Copy link
Contributor

  1. Enhance the SolidModel get_boundary method to optionally return only boundaries perpendicular to specified directions and only at the min/max position along the specified direction. This is useful if the user wishes to apply different boundary conditions on different exterior boundary faces.
  2. Introduce the set_periodic method to make the mesh on the specified groups periodic. Currently only supports 2D groups representing parallel axis-aligned surfaces (i.e. can only apply periodicity in the X, Y, or Z direction).

@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@simlapointe simlapointe marked this pull request as ready for review May 23, 2025 18:48
@simlapointe simlapointe requested a review from gpeairs May 23, 2025 18:48
Copy link
Member

@gpeairs gpeairs left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple minor comments!

end

"""
set_periodic!(group1::AbstractPhyiscalGroup, group2::AbstractPhyiscalGroup; dim=2)
Copy link
Member

Choose a reason for hiding this comment

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

"Phyiscal" typo x2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

end
end

function get_physical_group_bounding_box(dim, tags)
Copy link
Member

Choose a reason for hiding this comment

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

Just checking, tags should be entity tags, not physical group tags, right? I would just call it get_bounding_box.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, they should be entity tags. I renamed the function as you suggested.

Copy link
Member

Choose a reason for hiding this comment

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

Just remembered there's also a function SolidModels.bounds3d with a slightly different interface, oops. There's a docstring but it's not in the HTML docs yet (adding it as part of #58).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh my bad, thanks for catching that. I will remove get_bounding_box and use SolidModels.bounds3d instead. I'll open another PR for the change.

@simlapointe simlapointe force-pushed the simlapointe/sm-boundary-periodic branch from d7573a1 to 2f4d74f Compare May 23, 2025 22:11
Copy link
Member

@gpeairs gpeairs left a comment

Choose a reason for hiding this comment

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

LGTM

@gpeairs gpeairs merged commit 6865fdd into main May 24, 2025
6 checks passed
@simlapointe simlapointe deleted the simlapointe/sm-boundary-periodic branch May 30, 2025 16:48
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.

2 participants