Skip to content

Conversation

@ncorsobh
Copy link
Contributor

@ncorsobh ncorsobh commented Jul 11, 2025

Proposed changes

An earlier PR (#5960) had reordered the source and flux terms in ValenciaDivClean in an attempt to make the system better-behaved. However, it was empirically found that this reduced the quality of the magnetic field evolution. Additionally, convergence tests for higher order FD do not work properly unless this change is undone.

Upgrade instructions

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

Further comments

@jyoo1042 This PR should merge into develop a change that you are already using. Please confirm that this is indeed the case. And if so, this will formally implement it, so you can disregard the local change that you are using.

@jyoo1042
Copy link
Contributor

@ncorsobh yes. thank you!

@wthrowe
Copy link
Member

wthrowe commented Jul 11, 2025

Not a review of the code, but the "Upgrade instructions" are published in the documentation and release notes, so I don't think you actually want that content there.

@ncorsobh
Copy link
Contributor Author

Not a review of the code, but the "Upgrade instructions" are published in the documentation and release notes, so I don't think you actually want that content there.

Didn't realize this, thanks! I just moved the comment to the "Further comments" section. Hopefully that's better!

@wthrowe
Copy link
Member

wthrowe commented Jul 11, 2025

Yep, that's fine.

@nilsdeppe nilsdeppe merged commit eb2b94e into sxs-collaboration:develop Jul 14, 2025
24 checks passed
@ncorsobh ncorsobh deleted the revert_fluxes branch July 14, 2025 16:26
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.

5 participants