add non-conservative convection term

[qiauil]

• add non-conservative convection term
• allow to swift between conservative convection and non-conservative convection in steppers

[Ceyron]

Hi Qiang,

thanks for the updated PR. I left some remarks in the code.

[Ceyron]

A couple more general remarks:

Can you run the pre-commit hook, being in the same folder as the repo and then:

pip install pre-commit
pre-commit install --install-hooks
pre-commit run --all-files

This should resolve code-linting checks.

And please keep docstrings to 80 characters in length if possible

[Ceyron]

Thanks for the updates, some last stylistic requests:

1. In the _convection.py file: please add a newline under the **Arguments:** header, otherwise mkdocs will not correctly produce the documentation
2. Please use rewrap to ensure that the docstrings are at max 80 characters per line.

[qiauil]

Hi, Felix,

The new commit has been pushed; please take a look.

[Ceyron]

Thanks for all the work.

I just went over the code for a last time, and I think that the nonconservative eval functions (both for multi-channel and single-channel) should use the u_hat_dealiased for creating nabla_u in lines 167 and 228 of _convection.py. What do you think?

[qiauil]

Hi, Felix,

Good point!
To be honest, I am not sure about that. Do you think the linear operation will change the frequency of the field?
Theoretically, I would prefer to dealias the nabla_u as it is the actual one that does the nonlinear operation.

Anyway, I will currently modify the nabla_u term with the dealiased value; we could do some experiments later to see the difference.

[Ceyron]

The linear operator (i.e., the derivative_operator) does not move energy between the modes.

Aliasing occurs when we element-wise multiply two states in state space. To avoid creating any (undesirable) aliases, according to Orszag's 2/3 rule, both factors in the product must be zero-ed out in the upper 1/3 frequency range (which is what self. dealias does). Hence, we need to have both nabla_u and u being dealiased. At least, that's my current understanding of dealiasing. We can talk about it more in a separate thread.

With the modifications, I will now merge the PR. Thanks for the work :). If you are up for it, it would also be cool to extend some of the unit tests for the new four permutations of convection, but I'd say that's a separate PR.