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

Inject everything when assembling a MG hierarchy #3114

Closed
wants to merge 5 commits into from

Conversation

pbrubeck
Copy link
Contributor


name: "MG: Inject coefficients"
description: ''
title: ''
labels: ''
assignees: ''


Description

Things were breaking when a solver is reused for a bilinear form with a coefficient that is not a Constant , e.g. functions in R or DG0, after updating that coefficient.
Turns out that we also need to manually inject everything in problem.F.coefficients().
But wait, there's also another issue. The manual injection has to ensure that the level above the current one is up-to-date with respect to the finest level, an assumption that only holds in the two-level case. PCMG will traverse the levels in the order [1, 2, 3, ..., finest, 0 = coarsest]. So we should inject all coefficients across the hierarchy the first time we are reassembling the problem.

Fixes Issues:

  • TODO report the issue

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 changed the title Inject everything on level 1 Inject everything when assembling a MG hierarchy Sep 20, 2023
for xf, xc in zip(fine_coefficients, coarse_coefficients):
if xf not in seen:
manager.inject(xf, xc)
seen.add(xf)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it sufficient to only do this in kspcomputeoperators case, or should it also be done in form_jacobian and form_function? When we did this before (cc @pefarrell) we did it by setting up hooks master...pefarrell/transfer-manager-inject-everything

That was never merged because it does too much work (reinjects too frequently). I think this suffers from the same problem. There's no need to reinject a coefficient of F if it hasn't changed. These days you can ask the dat for its state, keep track of that and if it changed since the last time you saw it, you know its changed and you must inject the function.

@dham dham closed this Oct 11, 2023
@pbrubeck pbrubeck deleted the pbrubeck/fix/inject-coefficients branch November 25, 2024 10:31
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.

3 participants