Skip to content

Cjc/vp demo #4320

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 24 commits into
base: release
Choose a base branch
from
Open

Cjc/vp demo #4320

wants to merge 24 commits into from

Conversation

colinjcotter
Copy link
Contributor

Description

A 1D Vlasov Poisson demo

@colinjcotter colinjcotter requested review from JHopeCollins and dham May 14, 2025 10:24
JHopeCollins
JHopeCollins requested changes May 15, 2025
View reviewed changes
Copy link
Member

@JHopeCollins JHopeCollins left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, most of the comments are just extra detail where the explanations are brief, and a couple of small code changes to avoid warnings.

The only major suggestion is to reorder/reword the explanation of how the null space is dealt with.

demos/vlasov_poisson_1d/vp1d.py.rst
-\phi_{x_1x_1} = q_0\int f(x_1,x_2,t)\,\mathrm{d} x_2,

where :math:`\nabla=(\partial_{x_1},\partial{x_2})`. From now we will
choose units such that :math:`q_0,m` are absorbed into the definition of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a nondimensionalisation? Or simply rescaling f so it has different SI units?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the latter.

demos/vlasov_poisson_1d/vp1d.py.rst
Comment on lines +317 to +341
Each Runge-Kutta stage involves solving for :math:`\phi` before solving
for :math:`\partial f/\partial t`. Here is the first stage. ::

#
fstar.assign(fn)
phi_solver.solve()
df_solver.solve()
f1.assign(fn + df_out)

The second stage. ::

#
fstar.assign(f1)
phi_solver.solve()
df_solver.solve()
f2.assign(3*fn/4 + (f1 + df_out)/4)

The third stage. ::

#
fstar.assign(f2)
phi_solver.solve()
df_solver.solve()
fn.assign(fn/3 + 2*(f2 + df_out)/3)
t += dt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be written as a loop with the coefficient array defined beforehand? I don't think we want to be encouraging people to handcode unrolled Runge-Kutta loops.

@JHopeCollins
Copy link
Member

JHopeCollins commented May 15, 2025 •
edited
Loading

In the future, it could be nice to have a subsequent demo showing how to do this with a single solver, possibly with Irksome too.

The whole thing is linear, and if you have a mixed space with (phi, f) then the matrix is lower triangular so you can do exactly the method you have here with a multiplicative fieldsplit to first solve for phi at the current stage, then calculate the stage increment df using the latest potential velocity.
If you write this as a single system you can also pass it to Irksome to do the RK scheme.

@colinjcotter
Copy link
Contributor Author

Josh, the whole thing isn't linear, because of the a*f appearing in the conservation law.

colinjcotter and others added 7 commits May 15, 2025 15:09
colinjcotter and others added 2 commits May 15, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JHopeCollins JHopeCollins JHopeCollins requested changes

@dham dham Awaiting requested review from dham

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@colinjcotter @JHopeCollins @dham