Skip to content

Comments

Tensor product refinement#3

Closed
alexfikl wants to merge 26 commits intotensor-product-additionsfrom
tensor-product-refinement
Closed

Tensor product refinement#3
alexfikl wants to merge 26 commits intotensor-product-additionsfrom
tensor-product-refinement

Conversation

@alexfikl
Copy link
Owner

@alexfikl alexfikl commented Dec 6, 2020

@alexfikl alexfikl force-pushed the tensor-product-refinement branch from 5211cd5 to 25104db Compare December 12, 2020 03:30
Copy link

@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 for working on this. A few comments below. I think I'd like to merge this after inducer#70, just to keep that from becoming even larger than it already is.

# {{{ shape-dependent tesselation helpers

@singledispatch
def get_child_basis_vertex_indices(shape: mp.Shape, child):
Copy link

Choose a reason for hiding this comment

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

get_child_basis_vertex_indices doesn't make a great deal of sense to me as a name. What is it supposed to do? (Document, maybe rename?)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Removed this and its horrible name completely as it was just used in map_unit_nodes_to_children.

c7d09c9

Comment on lines 111 to 112
assert len(child) == shape.nvertices
return child[:shape.dim + 1]
Copy link

Choose a reason for hiding this comment

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

Does simplex-as-default make sense? (If I'm understanding this right?)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Removed this as it was only used in the map_unit_nodes_to_children function.

c7d09c9

#
# * lines and squares don't require any reordering

return [child[i] for i in [0, 1, 2, 4][:shape.dim + 1]]
Copy link

Choose a reason for hiding this comment

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

This sounds like it's limited to up to 3D. Add handling for larger-D cases.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Should be slightly more general after c7d09c9.

Comment on lines 227 to 230
from meshmode.mesh import _ModepyElementGroup
if not isinstance(group, _ModepyElementGroup):
raise TypeError(f"groups of type '{type(group.mesh_el_group).__name__}'"
" are not supported.")
Copy link

Choose a reason for hiding this comment

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

singledispatch?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Comment on lines 191 to 194
from meshmode.mesh import _ModepyElementGroup
if not isinstance(group, _ModepyElementGroup):
raise TypeError(f"groups of type '{type(group.mesh_el_group).__name__}'"
" are not supported.")
Copy link

Choose a reason for hiding this comment

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

singledispatch?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Comment on lines 93 to 97
if isinstance(meg, _ModepyElementGroup):
from meshmode.mesh.refinement.tesselate import map_unit_nodes_to_children
mapped_unit_nodes = map_unit_nodes_to_children(
meg._modepy_shape, fine_unit_nodes, record.tesselation)
else:
Copy link

Choose a reason for hiding this comment

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

singledispatch?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added in c7d09c9.

@alexfikl
Copy link
Owner Author

I think I'd like to merge this after inducer#70, just to keep that from becoming even larger than it already is.

I'm definitely fine with merging that first!

@alexfikl
Copy link
Owner Author

alexfikl commented Dec 16, 2020

@inducer Hm.. firedrake seems to be looking for an older pytools here. Is it passing on master?

EDIT: Guess it's incoming with OP2/PyOP2#605

@inducer
Copy link

inducer commented Dec 17, 2020

Yikes. Thanks for telling me about that. I had no idea they were using that. I guess bringing it back now won't help either.

inducer and others added 9 commits December 27, 2020 20:34
Increase terminate timeout in NodalDGContext

See merge request inducer/meshmode!106
Slightly loosen test_boundary_interpolation tolerance

See merge request inducer/meshmode!107
Co-authored-by: Andreas Klöckner <inform@tiker.net>
Co-authored-by: Andreas Klöckner <inform@tiker.net>
Co-authored-by: Andreas Klöckner <inform@tiker.net>
Co-authored-by: Andreas Klöckner <inform@tiker.net>
Co-authored-by: Andreas Klöckner <inform@tiker.net>
@alexfikl alexfikl force-pushed the tensor-product-refinement branch from df892c4 to da75962 Compare January 4, 2021 02:33
@alexfikl
Copy link
Owner Author

alexfikl commented Jan 4, 2021

Continued in inducer#97.

@alexfikl alexfikl closed this Jan 4, 2021
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