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

SparseGrid and Transfers for MPM #21753

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

xuchenhan-tri
Copy link
Contributor

@xuchenhan-tri xuchenhan-tri commented Jul 30, 2024

This change is Reviewable

Copy link
Contributor Author

@xuchenhan-tri xuchenhan-tri left a comment

Choose a reason for hiding this comment

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

+@amcastro-tri +@joemasterjohn for reviews please.

Please focus on things in the /multibody/mpm directory and ignore the changes in tools/.

I'd recommend starting from particles.* and sparse_grid.* and then work your way towards transfer.* while looking up definitions of helper classes/functions in auxiliary files along the way as needed.

Reviewable status: LGTM missing from assignees amcastro-tri,joemasterjohn, needs platform reviewer assigned, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @xuchenhan-tri)

Copy link
Contributor

@joemasterjohn joemasterjohn left a comment

Choose a reason for hiding this comment

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

Reviewed 39 of 56 files at r1.
Reviewable status: LGTM missing from assignees amcastro-tri,joemasterjohn, needs platform reviewer assigned, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @xuchenhan-tri)

Copy link
Contributor

@amcastro-tri amcastro-tri left a comment

Choose a reason for hiding this comment

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

Thanks @xuchenhan-tri!
FYI, Our highest priority with @joemasterjohn right now is to land margin to solve issues in Anzu. I am sorry, but our review of this will need to wait till next week. I believe that's not a blocker for you at this time. I'll review this ASAP after I'm done with the margin feature. Hopefully by end of week I can take a look or earlier next week.

Reviewable status: LGTM missing from assignees amcastro-tri,joemasterjohn, needs platform reviewer assigned, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @xuchenhan-tri)

Copy link
Contributor Author

@xuchenhan-tri xuchenhan-tri left a comment

Choose a reason for hiding this comment

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

Not a problem. I'm not blocked at the moment.

Reviewable status: LGTM missing from assignees amcastro-tri,joemasterjohn, needs platform reviewer assigned, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @xuchenhan-tri)

Copy link
Contributor

@amcastro-tri amcastro-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, LGTM missing from assignees amcastro-tri,joemasterjohn, needs platform reviewer assigned, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @xuchenhan-tri)

a discussion (no related file):
I am trying to run some of the tests and they don't even compile.
Maybe a rebase is needed?
Thanks!


Copy link
Contributor Author

@xuchenhan-tri xuchenhan-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, LGTM missing from assignees amcastro-tri,joemasterjohn, needs platform reviewer assigned, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @xuchenhan-tri)

a discussion (no related file):

Previously, amcastro-tri (Alejandro Castro) wrote…

I am trying to run some of the tests and they don't even compile.
Maybe a rebase is needed?
Thanks!

I disabled CI since we don't need it for this PR (not merging). Reverting the CI disabling commit locally should allow you to run the unit tests.


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

Successfully merging this pull request may close these issues.

4 participants