Skip to content

Generalization to 3D#96

Merged
amartinhuertas merged 2 commits intobulk_penalty_stabfrom
bulk_penalty_stab3D
Sep 2, 2024
Merged

Generalization to 3D#96
amartinhuertas merged 2 commits intobulk_penalty_stabfrom
bulk_penalty_stab3D

Conversation

@amboschman
Copy link
Copy Markdown
Collaborator

Selection of polytope type (lines 105-109) is now done via a conditional statement: is there a more elegant way to do this?

Comment thread bulk_ghost_penalty_canvas.jl
Comment thread bulk_ghost_penalty_canvas.jl Outdated
end
end
bounds = [(xmin[i], xmax[i]) for i in 1:D]
point_iterator = Iterators.product([bounds[i] for i in 1:D]...)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can't you just use here Iterators.product(bounds...) ?

Comment thread bulk_ghost_penalty_canvas.jl Outdated
bounds = [(xmin[i], xmax[i]) for i in 1:D]
point_iterator = Iterators.product([bounds[i] for i in 1:D]...)
bounding_box_node_coords[bounding_box_node_ids[agg]] =
reduce(vcat,[Point(p...) for p in point_iterator])
Copy link
Copy Markdown
Member

@amartinhuertas amartinhuertas Aug 30, 2024

Choose a reason for hiding this comment

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

I think a less memory hungry version of

reduce(vcat,[Point(p...) for p in point_iterator])

would be to use

reshape([Point(p...) for p in point_iterator],2^D)

Again ... not a big deal ...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense, thanks!

Comment thread bulk_ghost_penalty_canvas.jl Outdated
Copy link
Copy Markdown
Member

@amartinhuertas amartinhuertas left a comment

Choose a reason for hiding this comment

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

hi @amboschman ... great work! Just minor comments for improvement (mainly so that we also get familiarized with the workflow).

implemented suggested changes (by Alberto on Generalization to 3D #96)
Copy link
Copy Markdown
Member

@amartinhuertas amartinhuertas left a comment

Choose a reason for hiding this comment

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

Thanks!

@amartinhuertas amartinhuertas merged commit 07c174e into bulk_penalty_stab Sep 2, 2024
@amboschman amboschman deleted the bulk_penalty_stab3D branch September 2, 2024 01:53
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