Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add regularization to Laplacian and preconditioners for
ConjugateGradientPoissonSolver#3848base: main
Are you sure you want to change the base?
Add regularization to Laplacian and preconditioners for
ConjugateGradientPoissonSolver#3848Changes from 9 commits
4f6ffd719f0d68b62951cd382923f132a5f42c3993b667af0a12c2f0c6535423f62872e9af75a130a667b1db28786dee35File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to mask
Lϕafter this operation? It seems to that the masked region also gets a non-zero value after this line.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good catch. Actually, while
dotandnormshould ignore immersed regions, I'm actually not sure they do right now so this may matter. I will test it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be clearer to call this the
regularization_constant. What do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
regularizer?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a stupid question. It seems to me that
regularizationandδis set to a positive number, 1 for example in the demo. However, the Laplacian operator is actually negative semi-definite (although our previous conversations were about positive semi-definite), and thusregularization(or sayδ) should be negative as well. Am I missing something?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, right... I played around this and found that the solver "doesn't blow up" only if we use positive
regularization. But this may actually be a clue that there's a bug somewhere.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now convinced that there is a bug or some conceptual error given that this doesn't work with$\delta < 0$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have found why the solver blows up for positive
regularization.Here,
pis the solution in the whole grid by the FFT Poisson solver, butmean_pcalculates the mean ofpin the immersed boundary grid. In my test,mean_pis usually order 1e-5, which is much larger than round-off errors. I also calculated the eigenvalues of the preconditioner withregularization=0, and I found both negative and positive eigenvalues.Removing the subtraction of
mean_penables convergence for positiveregularizationaccording to my test.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is interesting! I'll test it. I don't completely understand why this is what's needed, but let's think about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still getting NaNs. What
regularizationdid you use? Did you make any other changes? EDIT: the regularization does seem to matter, now I get non-blow-up with regularization ~ 1/L^2.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the code in a simpler case without
FlatExtrapolationOpenBoundaryCondition. That may explain why the result is different. Besides, I usually check the distribution of eigenvalues to judge whether the solver is numerically stable than running the simulation. I am looking into the case withFlatExtrapolationOpenBoundaryConditionand aim to understand how it changes the distribution of eigenvalues.I just ran your demo and got
NaNsas well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to write out the idea, we are trying to solve a "regularized" Poisson equation:
where$\tilde L = \nabla^2 - \delta V^{-1} \int \mathrm{d V}$ is the regularized Laplace operator (on an immersed boundary grid) with the singularity removed.
We're proposing to use a preconditioner that directly inverts (with an FFT --- fast) the "non-immersed" (but regularized) Laplace operator,
where$\nabla_u^2$ applies on the "underlying grid" (ie ignoring the immersed boundary). The present discussion is how to define the mean, which is taken over the domain $\Omega_\star$ with volume $V_\star$ . The current code takes $V_\star = V$ , but we can also consider taking $V_\star = V_u$ and $\Omega_\star = \Omega_u$ .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way @Yixiao-Zhang I think this example really meaningfully exposes the effect of regularization, because it's only with this kind of open boundary condition that$\bar R \ne 0$ . I believe that with impenetrable (and periodic of course) boundary conditions $\bar R$ is very small.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not realized$\overline{R} \neq 0$ for this kind of open boundary condition. However, I think the current pressure solver cannot handle $\hat{n}\cdot \nabla r = 0$ , but it should be $(\hat{n}\cdot \nabla)^2 r = 0$ for open boundary conditions? (I am not sure.)
FlatExtrapolationOpenBoundaryConditionproperly. As far as I understand, the pressure solver is unaware of the open boundary condition, but the open boundary condition should affect howfill_halo_region!works forrhs. The original boundary condition forrhsisRight now,
rhsis defined in this way:The good news is that the Laplace operator becomes negative definite if we take the open boundary into account. So, there is no need to regularize the Poisson solver. (The preconditioner is still semi-definite, though.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't quite true. The inhomogeneous part of any boundary condition is incorporated into the Poisson equation RHS (this happens implicitly by 1) filling halo regions on the predictor velocity prior to the pressure solve and then 2) taking the divergence of the predictor velocity.) This is why the operator / FFT-solve must use homogeneous Neumann conditions for the algorithm to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle, the preconditioner solves the Poisson equation in the underlying grid. Following this logic, it is the mean of
rhscalculated in the underlying grid that should be subtracted. I have not tested which is better, but I feel the mean in the underlying grid is more consistent mathematically. (I assumemean(rhs)here is the mean ofrhscalculated in the immersed boundary grid.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct,
mean(rhs)accounts for the immersed boundary. My objective with this was to define a preconditioner that (approximately) solves the same problem as the CG iteration. This means usingmeanover the whole grid.Put another way, if we use
meanover the whole grid, then the mean pressure produced by the preconditioner would be different than the mean pressure that would minimize the conjugate gradient residual?It's worth testing though. To implement this properly, we need to know the volume of the immersed region which will require an additional computation.