Skip to content

Rayleigh-Benard demo using Irksome and patch smoothing #4319

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

Open
wants to merge 14 commits into
base: release
Choose a base branch
from

Conversation

rckirby
Copy link
Contributor

@rckirby rckirby commented May 14, 2025

Description

Add a demo using Irksome for a time-dependent Rayleigh-Benard convection problem. We use a monolithic multigrid scheme with Vanka patch relaxation for the time stepping.

@rckirby rckirby requested a review from pbrubeck May 14, 2025 10:45
tends to make prettier pictures for low Rayleigh numbers, but also
tends to take more Newton iterations since the coupling terms in the
Jacobian are a bit stronger. Switching to the first case would be a
simple change of bits of the boundary associated with the second and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
simple change of bits of the boundary associated with the second and
simple change of the boundary subdomains associated with the second and

@rckirby rckirby requested review from ScottMacLachlan and dham May 14, 2025 11:08
@dham dham marked this pull request as ready for review May 14, 2025 12:04
Copy link
Contributor

@ScottMacLachlan ScottMacLachlan left a comment

Choose a reason for hiding this comment

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

I have a few minor suggestions on the demo itself. However, there seems to be a lot of unrelated content in this PR - did a merge go awry somewhere?


base_msh = UnitSquareMesh(Nbase, Nbase)
mh = MeshHierarchy(base_msh, ref_levels)
msh = mh[-1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want a comment here as to why we're using a MeshHierarchy?


Like Navier-Stokes, the pressure is only defined up to a constant.::

nullspace = [(1, VectorSpaceBasis(constant=True))]
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks wrong to me - we have 3 spaces in Z, but only 2 in the nullspace...

butcher_tableau = RadauIIA(num_stages)
We are going to carry out time stepping via Irksome, but we need
to say how to solve the rather interesting stage-coupled system. ::
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we should add more words about the details of the solver. There aren't a lot of demos with MMG, so explaining what we do would be worthwhile.

"ksp_type": "fgmres",
"ksp_monitor": None,
"ksp_max_it": 200,
"ksp_atol": 1.e-12,
Copy link
Contributor

Choose a reason for hiding this comment

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

You're using Eisenstat-Walker, so why also set ksp_atol?

"ksp_monitor": None,
"ksp_max_it": 200,
"ksp_atol": 1.e-12,
"ksp_gmres_restart": 30,
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems so wrong - why bother with a restart so small?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants