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

DiscretizationCollection constructor should do no work #104

Open
inducer opened this issue May 27, 2021 · 3 comments
Open

DiscretizationCollection constructor should do no work #104

inducer opened this issue May 27, 2021 · 3 comments
Labels
design decisions Anything related to overhauling or redesigning code

Comments

@inducer
Copy link
Owner

inducer commented May 27, 2021

This is too fat. The constructor itself should just be an attribute setter. The actual work should be done by a "maker function", like make_discretization_collection. This is a long-term thing that'll hopefully make it easier to move towards moving geometry.

x-ref inducer/meshmode#123

@thomasgibson
Copy link
Collaborator

What's the cleanest way to do this without breaking a whole bunch of stuff? We need to support dcoll = DiscretizationCollection(...)? Should we introduce a function alias? Or should we take the opportunity to use PRs like illinois-ceesd/mirgecom#360 to make sure this get's taken care of? ATM our main worry is mirgecom in terms of compatibility.

@thomasgibson
Copy link
Collaborator

thomasgibson commented May 27, 2021

Actually, I think we should do this after EagerDiscretization is completely removed from mirgecom. After taking a cursory look at what is entailed, I think this will not be an easy refactoring.

In order to remove all work from DiscretizationCollection.__init__(...), you need to split out the following:

def _set_up_distributed_communication(self, mpi_communicator, array_context):

def connection_from_dds(self, from_dd, to_dd):

def discr_from_dd(self, dd):

and several other related private method (the ones I listed are the really big ones)

@thomasgibson thomasgibson added the design decisions Anything related to overhauling or redesigning code label May 27, 2021
@inducer
Copy link
Owner Author

inducer commented May 28, 2021

My thought is that this won't be cataclysmic. Most of the work could be moved to functions that the constructor calls while complaining, whereas the canonical call sites would be in make_discretization_collection or some such. I don't know why connection_from_dd or discr_from_dd would need to move though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design decisions Anything related to overhauling or redesigning code
Projects
None yet
2 participants