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

Fix coarsening for R space #3814

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix coarsening for R space #3814

wants to merge 2 commits into from

Conversation

pbrubeck
Copy link
Contributor

@pbrubeck pbrubeck commented Oct 21, 2024

Description

This PR fixes two separate issues for multigrid with Functions in R.

  1. The restriction operator for the R space is square. For these cases PETSc complains if we do not pass a vector to rescale the restriction operator to construct the "cheap" injection, even though it is not necessary, as we already pass the proper injection operator. To fix this we just pass a vector of the appropriate size.

  2. The parentdm of an R space function sometimes might have a null __setup_hooks__, which makes it impossible to call add_hook on it. As a workaround, we just skip that call.

@@ -384,7 +385,8 @@ def create_interpolation(dmc, dmf):
mat.setType(mat.Type.PYTHON)
mat.setPythonContext(ctx)
mat.setUp()
return mat, None
rscale = mat.createVecLeft() if row_size == col_size else None
Copy link
Contributor Author

@pbrubeck pbrubeck Oct 21, 2024

Choose a reason for hiding this comment

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

Not sure whether Right or Left is the correct thing here. But it does not really matter if the sizes are the same.

Copy link

github-actions bot commented Oct 21, 2024

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake complex8079 ran6497 passed1582 skipped0 failed

Copy link

github-actions bot commented Oct 21, 2024

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake real8085 ran7299 passed786 skipped0 failed

@pbrubeck pbrubeck force-pushed the pbrubeck/fix/coarsen-dm branch from 292191a to de93ebc Compare October 21, 2024 20:30
@pbrubeck pbrubeck force-pushed the pbrubeck/fix/coarsen-dm branch from de93ebc to 0d1e1e8 Compare October 21, 2024 21:38

# This test will fail if we raise this warning
with warnings.catch_warnings():
warnings.filterwarnings("error", "Creating new TransferManager", RuntimeWarning)
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks very fragile. If we decided not to emit this warning any more, or even changed the content of the warning message, then this would always pass regardless of whether or not we were actually reusing transfer managers.

Is a better way to do this not to somehow retrieve the transfer managers from whatever mesh/DM they are attached to and check that they are the same?

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