Skip to content
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

Time-dependent demo (bubble shear) #158

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Time-dependent demo (bubble shear) #158

wants to merge 2 commits into from

Conversation

ddundo
Copy link
Member

@ddundo ddundo commented Feb 20, 2025

Closes #157.

Based on Animate's bubble shear demo.

@ddundo ddundo added documentation Improvements or additions to documentation PRIORITY We should address this ASAP labels Feb 20, 2025
@ddundo ddundo requested a review from jwallwork23 February 20, 2025 16:41
Comment on lines 98 to 101
try:
H_interp.interpolate(H)
except NameError:
H_interp.interpolate(Constant(((1.0, 0.0), (0.0, 1.0))))
Copy link
Member Author

@ddundo ddundo Feb 20, 2025

Choose a reason for hiding this comment

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

I had to add this try-except because MongeAmpereMover calls monitor during initialisation. Here's the error without it:

Traceback (most recent call last):
  File "/home/firedrake/firedrake/src/movement/demos/bubble_shear.py", line 114, in <module>
    mover = MongeAmpereMover(mesh, monitor, method="quasi_newton", rtol=1e-8)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/firedrake/firedrake/src/movement/movement/monge_ampere.py", line 87, in MongeAmpereMover
    return implemented_methods[method](mesh, monitor_function, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "petsc4py/PETSc/Log.pyx", line 188, in petsc4py.PETSc.Log.EventDecorator.decorator.wrapped_func
  File "petsc4py/PETSc/Log.pyx", line 189, in petsc4py.PETSc.Log.EventDecorator.decorator.wrapped_func
  File "/home/firedrake/firedrake/src/movement/movement/monge_ampere.py", line 586, in __init__
    super().__init__(mesh, monitor_function=monitor_function, **kwargs)
  File "/home/firedrake/firedrake/src/movement/movement/monge_ampere.py", line 148, in __init__
    super().__init__(mesh, monitor_function=monitor_function, **kwargs)
  File "/home/firedrake/firedrake/src/movement/movement/mover.py", line 77, in __init__
    self._create_functions()
  File "/home/firedrake/firedrake/src/movement/movement/monge_ampere.py", line 601, in _create_functions
    super()._create_functions()
  File "/home/firedrake/firedrake/src/movement/movement/monge_ampere.py", line 181, in _create_functions
    self.monitor.interpolate(self.monitor_function(self.mesh))
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/firedrake/firedrake/src/movement/demos/bubble_shear.py", line 99, in monitor
    H_interp.interpolate(H)
                         ^
NameError: name 'H' is not defined. Did you mean: 'H1'?

Comment on lines 114 to 118
mover = MongeAmpereMover(mesh, monitor, method="quasi_newton", rtol=1e-8)
c_mov = get_initial_condition(mover.mesh)
movement_simulation = run_simulation(mover.mesh, 0.0, simulation_end_time, c_mov)
t = 0
H = RiemannianMetric(TensorFunctionSpace(mover.mesh, "CG", 1))
Copy link
Member

Choose a reason for hiding this comment

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

Would this not fix your issue? Ensure H exists before it's needed.

Suggested change
mover = MongeAmpereMover(mesh, monitor, method="quasi_newton", rtol=1e-8)
c_mov = get_initial_condition(mover.mesh)
movement_simulation = run_simulation(mover.mesh, 0.0, simulation_end_time, c_mov)
t = 0
H = RiemannianMetric(TensorFunctionSpace(mover.mesh, "CG", 1))
H = RiemannianMetric(TensorFunctionSpace(mover.mesh, "CG", 1))
mover = MongeAmpereMover(mesh, monitor, method="quasi_newton", rtol=1e-8)
c_mov = get_initial_condition(mover.mesh)
movement_simulation = run_simulation(mover.mesh, 0.0, simulation_end_time, c_mov)
t = 0

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Although I left this in more as a reminder to ask you whether we really need to call the monitor function during initialisation, since it gets called again during mover.move(). Seems like it could be optimised.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh and maybe not, since RiemannianMetric (and the solution) needs to be defined on mover.mesh - I think?

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, of course - apologies.

@ddundo
Copy link
Member Author

ddundo commented Feb 21, 2025

Just to clarify what I meant during the meeting when I suggested passing args/kwargs to the monitor function:

def monitor(mesh, solution):
   ...

def run_simulation(...):
    ...
    mover = MongeAmpereMover(...)
    while float(t) < t_end + 0.5 * float(dt):
        lvs.solve()  # solve for c
        mover.move(c)  # pass c to monitor

Or we could do it more explicitly, for example:

monitor_kwargs = {"solution": c}
mover.move(**monitor_kwargs)

@jwallwork23
Copy link
Member

Just to clarify what I meant during the meeting when I suggested passing args/kwargs to the monitor function:

def monitor(mesh, solution):
   ...

def run_simulation(...):
    ...
    mover = MongeAmpereMover(...)
    while float(t) < t_end + 0.5 * float(dt):
        lvs.solve()  # solve for c
        mover.move(c)  # pass c to monitor

Or we could do it more explicitly, for example:

monitor_kwargs = {"solution": c}
mover.move(**monitor_kwargs)

Thanks. I guess one question I have is why do we need to define get_initial_condition, velocity_expression, and run_simulation if they're only called once? For the purposes of a demo, couldn't we just inline them into the main program? Then we wouldn't need to change the signature of the monitor function.

@ddundo
Copy link
Member Author

ddundo commented Feb 21, 2025

Thanks. I guess one question I have is why do we need to define get_initial_condition, velocity_expression, and run_simulation if they're only called once? For the purposes of a demo, couldn't we just inline them into the main program? Then we wouldn't need to change the signature of the monitor function.

You're right, no need to complicate it for now. I inlined them in ac24b50. Still fiddly with the NameError though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation PRIORITY We should address this ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

time dependent example?
2 participants