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

Correct number of candidates for prolong/restrict on extruded meshes #3148

Merged
merged 3 commits into from
Oct 18, 2023

Conversation

pbrubeck
Copy link
Contributor

@pbrubeck pbrubeck commented Oct 10, 2023


name: "Bug fix or new feature"
description: ''
title: ''
labels: ''
assignees: ''


Description

Prolongation and restriction kernels were incorrectly looping into out of bounds coarse basis functions in the extruded case. The logic added in 86e6ca3 was only correct for the injection kernel. So far the tests only considered cases that terminated the loop before going out of bounds. When the children cells fall out of the parent coarse cell, we access out of bounds memory, which utimately causes a nan to be returned on these cases.

I also added a test with a deformed mesh hierarchy, although inject still breaks without producing nan for degree > 1 and only on some types of cells (works on interval x interval).

Associated Pull Requests:

  • Make a list (with links!)

Fixes Issues:

  • Make a list of issues (with links!)

If issues are fixed by this PR, prepend each of them with the word
"fixes", so they are automatically closed when this PR is merged. For
example "fixes #123, fixes #456".

Checklist for author:

  • I have linted the codebase (make lint in the firedrake source directory).
  • My changes generate no new warnings.
  • All of my functions and classes have appropriate docstrings.
  • I have commented my code where its purpose may be unclear.
  • I have included and updated any relevant documentation.
  • Documentation builds locally (make linkcheck; make html; make latexpdf in firedrake/docs directory)
  • I have added tests specific to the issues fixed in this PR.
  • I have added tests that exercise the new functionality I have introduced
  • Tests pass locally (pytest tests in the firedrake source directory) (useful, but not essential if you don't have suitable hardware).
  • I have performed a self-review of my own code using the below guidelines.

Checklist for reviewer:

  • Docstrings present
  • New tests present
  • Code correctly commented
  • No bad "code smells"
  • No issues in parallel
  • No CI issues (excessive parallelism/memory usage/time/warnings generated)
  • Upstream/dependent branches and PRs are ready

Feel free to add reviewers if you know there is someone who is already aware of this work.

Please open this PR initially as a draft and mark as ready for review once CI tests are passing.

@pbrubeck pbrubeck requested a review from wence- October 10, 2023 12:28
@ksagiyam
Copy link
Contributor

So what was level_ratio?

@pbrubeck
Copy link
Contributor Author

So what was level_ratio?

It's simply the ratio of the column height of the fine and coarse meshes.

@dham dham merged commit 941a6d3 into master Oct 18, 2023
@dham dham deleted the pbrubeck/fix/mg-extruded-candidates branch October 18, 2023 15:58
Ig-dolci pushed a commit that referenced this pull request Oct 20, 2023
…3148)

* Correct number of candidates for prolong/restrict on extruded meshes
* Test high order prolongation/restriction
pbrubeck added a commit that referenced this pull request Oct 25, 2023
…3148)

* Correct number of candidates for prolong/restrict on extruded meshes
* Test high order prolongation/restriction
pbrubeck added a commit that referenced this pull request Oct 25, 2023
…3148)

* Correct number of candidates for prolong/restrict on extruded meshes
* Test high order prolongation/restriction
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.

Support for local_range
3 participants