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

Update DOFArray flatten and unflatten #275

Merged
merged 5 commits into from
Oct 23, 2021

Conversation

alexfikl
Copy link
Collaborator

Following up on inducer/arraycontext#91 (comment).

At the moment this just updates the docs in the dof_array flattening functions to also mention the ones in arraycontext. There's a few of these: flatten, unflatten, unflatten_like, flatten_to_numpy and unflatten_from_numpy; and all of them just work on DOFArrays and leave the rest of the container alone.

How many of these should be deprecated? At least the numpy ones have some nice uses

  • in some tests, but these can probably be replaced with some reshaping, e.g.
nodes = thaw(discr.nodes(), actx)
nodes = np.stack(flatten_to_numpy(nodes, actx))
  • pytential uses unflatten_from_numpy in a couple of places. Not sure any of those actually flatten anything that's not a DOFArray though.

  • pytential also uses flatten quite a lot to talk to sumpy. This could be replaced by arraycontext.flatten + reshape too, to make it (3, nparticles), I think, since sumpy supports both types of inputs.

@inducer
Copy link
Owner

inducer commented Oct 20, 2021

I don't see a principled reason to keep any of them. They seem kind of redundant now that there's a documented way to flatten given just an array context.

@alexfikl alexfikl marked this pull request as ready for review October 21, 2021 01:32
@alexfikl
Copy link
Collaborator Author

I don't see a principled reason to keep any of them. They seem kind of redundant now that there's a documented way to flatten given just an array context.

At least for the tests in meshmode it was impressively simple to replace them with the arraycontext versions 🎉. Should we also remove them from the docs?

@inducer
Copy link
Owner

inducer commented Oct 22, 2021

Yep, I say let's remove them from the docs as well.

@alexfikl
Copy link
Collaborator Author

Yep, I say let's remove them from the docs as well.

Removed in 783686d.

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! Spotted one wrinkle, otherwise LGTM!

meshmode/dof_array.py Show resolved Hide resolved
@inducer
Copy link
Owner

inducer commented Oct 22, 2021

Thx!

@inducer inducer merged commit a50fb23 into inducer:main Oct 23, 2021
@alexfikl alexfikl deleted the dof-flatten-docs branch January 17, 2022 15:38
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