add non-conservative form for convection operator #45
Conversation
exponax/nonlin_fun/_convection.py
Outdated
| ) | ||
| u_hat_dealiased = self.dealias(u_hat) | ||
| u = self.ifft(u_hat_dealiased) | ||
| conv_u=sum( |
There was a problem hiding this comment.
Could you use a broadcasted version for the contraction for efficiency.
There was a problem hiding this comment.
For readability, I could imagine that it's better to have a separate line in which you compute the velocity gradient in Fourier space.
| for 1D and | ||
|
|
||
| ``` | ||
| 𝒩(u) = b₁ ∇ ⋅ (u ⊗ u) |
There was a problem hiding this comment.
Could you tell me what the reason was for removing the 1/2? Now, I think that the conservative and nonconservative no longer match with each other.
|
Two more general points:
|
|
Hi, Felix, I have uploaded the code and modified the steppers. Please let me know if you have any further comments. Cheers, |
| ): | ||
| """ | ||
| Performs a pseudo-spectral evaluation of the nonlinear convection, e.g., | ||
| found in the Burgers equation. In 1d and state space, this reads | ||
|
|
||
| ``` | ||
| 𝒩(u) = - b₁ 1/2 (u²)ₓ | ||
| 𝒩(u) = b₁ u (u)ₓ |
There was a problem hiding this comment.
Hi Qiang,
I think we should keep the minus here, right?
There was a problem hiding this comment.
Yes! That was my mistake... all the minus should be kept here.
There was a problem hiding this comment.
Great 👍
Could you add them again in the PR?
|
Hi Qiang, The docstring is still without the minus. Did you forget to push your changes? |
|
Hi, Felix I think it should be a GitHub issue as it shows "Checking for the ability to merge automatically…Hang in there while we check the branch’s status." from my side since I pushed the commit. Can you see my other changes, e.g., adding codes for single-channel non-conservative convection? Sorry to hear you were sick; get well soon :) |
|
Hi, Felix, I just pushed a new commit with a minor change, but it still shows "processing updates" here. |
|
I think it could be related to the fact that I made the repo public on Friday. |
|
Hi, Felix, Good point! I just changed the visibility and pushed again. But it seems not working. I just have a search, and it seems to be a common issue; see https://github.com/orgs/community/discussions/78775, and we might need to add a new pull request if it is still not working later... |
|
Hmmm, yes, recreating the PR seems to be the easiest solution. |
|
New PR is #46 |
Modifications:
_multi_channel_evalfunc to_multi_channel_conservative_evaland remove the 0.5 coefficient.