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

Ksagiyam/submesh core #3478

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

Ksagiyam/submesh core #3478

wants to merge 18 commits into from

Conversation

ksagiyam
Copy link
Contributor

@ksagiyam ksagiyam commented Mar 27, 2024

Enable cell submesh.

Depends on:

UFL

@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 3 times, most recently from 9b8f70c to 1e7c035 Compare April 4, 2024 01:07
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 10 times, most recently from 219240f to d555615 Compare April 11, 2024 12:19
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 2 times, most recently from 9b1d48b to 23caf2e Compare April 17, 2024 21:14
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 7 times, most recently from f5afedb to 824f2a5 Compare May 7, 2024 14:57
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 2 times, most recently from 76e89ef to 7e3c248 Compare May 15, 2024 22:16
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch from b6a41e7 to 9c839e0 Compare May 27, 2024 13:20
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 2 times, most recently from 1167181 to fc6545c Compare June 4, 2024 19:41
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch from a76cca5 to 630bfb4 Compare June 10, 2024 21:49
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch from 2fe02f9 to 8ba65d6 Compare June 26, 2024 21:36
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch from e204069 to b8cc6f6 Compare July 12, 2024 09:49
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 2 times, most recently from 4958817 to 7cda1d3 Compare December 11, 2024 20:39
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 4 times, most recently from da2063b to 1b654c1 Compare December 22, 2024 13:32
Comment on lines +288 to +290
# This way of caching is fragile.
# Should Implement _hash_key_() for ModifiedTerminal and use the entire mt as key.
return (ufl_element, mt.local_derivatives, ctx.point_set, ctx.integration_dim, entity_id, coordinate_element, mt.restriction, domain._ufl_hash_data_())
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this key should not depend on the mesh. We just need to call finat once, independently of mt.terminal (and its domain).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CoordinateMapping uses mt.terminal:

    @serial_cache(hashkey=make_basis_evaluation_key)
    def basis_evaluation(self, finat_element, mt, entity_id):
        return finat_element.basis_evaluation(mt.local_derivatives,
                                              self.point_set,
                                              (self.integration_dim, entity_id),
                                              coordinate_mapping=CoordinateMapping(mt, self))

Copy link
Contributor

Choose a reason for hiding this comment

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

But internally it only cares about the element

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably make CoordinateMapping only depend on the UFL element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In concept that might be true, but we probably need refactoring. CoordinateMapping also has:

    def cell_size(self):
        return self.interface.cell_size(self.mt.restriction)

which implicitly depends on domain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either way, as long as we pass mt to basis_evaluation, we need to use the entire mt in the cache key to ensure that we can safely use mt.something_new in the relevant parts of the code.

@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch 4 times, most recently from 0ff324a to a3a573d Compare January 7, 2025 16:53
@ksagiyam ksagiyam force-pushed the ksagiyam/submesh_core branch from 8dcdac2 to 474fa5d Compare January 10, 2025 02:35
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