Runtime BC control for both cc and fc variables

[chongchonghe]

Description

Now, we define boundary conditions with a single runtime parameter. This overrides geometry.is_periodic = 1 1 1. No need for BCs_cc in the problem creator.

quokka.bc = periodic periodic reflecting

Checklist

Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an x inside the square brackets [ ] in the Markdown source below:

• I have added a description (see above).
• I have added a link to any related issues (if applicable; see above).
• I have read the Contributing Guide.
• I have added tests for any new physics that this PR adds to the code.
• (For quokka-astro org members) I have manually triggered the GPU tests with the magic comment /azp run.

[chongchonghe]

@BenWibking I'll wait for your comments before refactoring all the problems to use this new design, which requires some python scripting.

[BenWibking]

This is a nice clean-up. Can it also set BCs_fc?

[chongchonghe]

This is a nice clean-up. Can it also set BCs_fc?

Not yet, but it's trivial to extend it to support that. Then, there will be a quokka.BC_fc

[BenWibking]

This is a nice clean-up. Can it also set BCs_fc?

Not yet, but it's trivial to extend it to support that. Then, there will be a quokka.BC_fc

I think it would be better to set both with the same parameter. Is there a case where you think that wouldn't be possible?

[chongchonghe]

This is a nice clean-up. Can it also set BCs_fc?

Not yet, but it's trivial to extend it to support that. Then, there will be a quokka.BC_fc

I think it would be better to set both with the same parameter. Is there a case where you think that wouldn't be possible?

That will be even simpler. I don't know if MHD B.C. will always be the same as hydro. If yes, then we should use the same parameter.

What about reflecting BC for fc variables? All current MHD tests use periodic or Dirichlet BC, but I think we want to support reflecting BC as well, right? Do we have to handle odd/even reflection, or is it always even reflection since we only have one variable (magnetic field) for each dimension?

[BenWibking]

This is a nice clean-up. Can it also set BCs_fc?

Not yet, but it's trivial to extend it to support that. Then, there will be a quokka.BC_fc

I think it would be better to set both with the same parameter. Is there a case where you think that wouldn't be possible?

That will be even simpler. I don't know if MHD B.C. will always be the same as hydro. If yes, then we should use the same parameter.

What about reflecting BC for fc variables? All current MHD tests use periodic or Dirichlet BC, but I think we want to support reflecting BC as well, right? Do we have to handle odd/even reflection, or is it always even reflection since we only have one variable (magnetic field) for each dimension?

You can do reflecting BCs for magnetic fields, but you have to handle the parity in a special way. The normal component should be even, and the transverse components should be odd.

[chongchonghe]

You can do reflecting BCs for magnetic fields, but you have to handle the parity in a special way. The normal component should be even, and the transverse components should be odd.

@BenWibking I'll leave this for a future PR. In this PR, I assume 'reflecting' for cc = 'reflect_even' for fc, just like what you did in all the MHD particle tests.

[chongchonghe]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[chongchonghe]

@BenWibking This is ready for review.

[chongchonghe]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[chongchonghe]

@BenWibking This is ready.

[markkrumholz]

Just left some minor cleanup comments.

[markkrumholz]

Looks like this will need to be updated to the base branch to pass the rocm-diskgalaxy-container test. Also, there's still a SonarCloud failure that will block merging.

[chongchonghe]

Also, there's still a SonarCloud failure that will block merging.

The SonarCloud warnings are optional and we can ignore it. I think unless we can control the rules of SonarCloud, we should not make it mandatory. We had discussions in #1529

[sonarqubecloud]

Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE