-
Notifications
You must be signed in to change notification settings - Fork 13
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
Enable adjoint #607
base: main
Are you sure you want to change the base?
Enable adjoint #607
Changes from all commits
eb360ac
93ad1cf
b54f3bd
998deb6
209d056
1682f88
e49b006
2b38eea
b0055c8
a7e6de1
3bc9a15
4cfd4bb
7b9d24d
31d5c59
96becfe
9f9b8eb
61ca2dc
6da7518
14371fd
9b8b0ed
b088504
7680ebc
af8908b
c6eb3b2
719a194
542d33e
efc7e99
f89173a
84814ce
06de329
4331a01
49b5e76
ac6cd36
d2fe122
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -1,7 +1,7 @@ | ||||
"""Some simple tools for configuring the model.""" | ||||
from abc import ABCMeta, abstractproperty | ||||
from abc import ABCMeta, abstractmethod | ||||
from enum import Enum | ||||
from firedrake import sqrt, Constant | ||||
from firedrake import sqrt, Constant, Function, FunctionSpace | ||||
|
||||
|
||||
__all__ = [ | ||||
|
@@ -12,7 +12,7 @@ | |||
"EmbeddedDGOptions", "ConservativeEmbeddedDGOptions", "RecoveryOptions", | ||||
"ConservativeRecoveryOptions", "SUPGOptions", "MixedFSOptions", | ||||
"SpongeLayerParameters", "DiffusionParameters", "BoundaryLayerParameters", | ||||
"SubcyclingOptions" | ||||
"SubcyclingOptions", "convert_parameters_to_real_space" | ||||
] | ||||
|
||||
|
||||
|
@@ -167,7 +167,7 @@ class ShallowWaterParameters(Configuration): | |||
class WrapperOptions(Configuration, metaclass=ABCMeta): | ||||
"""Base class for specifying options for a transport scheme.""" | ||||
|
||||
@abstractproperty | ||||
@abstractmethod | ||||
def name(self): | ||||
pass | ||||
|
||||
|
@@ -308,3 +308,17 @@ def check_options(self): | |||
raise ValueError( | ||||
"Cannot provide both fixed_subcycles and subcycle_by_courant" | ||||
+ "parameters.") | ||||
|
||||
|
||||
def convert_parameters_to_real_space(parameters, mesh): | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naive question, but can we do this conversion from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i.e. could we do that conversion here: gusto/gusto/core/configuration.py Line 87 in d2fe122
Constant s? Can we just do straight to real functions?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the problem that we need the mesh? Maybe we just pass that to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, the problem is that we need the mesh. I think it probably would be better to pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Obviously that unfortunately would make this a bigger change as all examples would need updating! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think that might be why we didn't want to do it... it does feel like a better solution though |
||||
"""Convert parameters to functions in real space. | ||||
|
||||
Args: | ||||
parameters (:class:`Configuration`): the configuration object | ||||
containing the parameters to convert | ||||
mesh (:class:`firedrake.Mesh`): the mesh object to use for the real space. | ||||
""" | ||||
R = FunctionSpace(mesh, 'R', 0) | ||||
for name, value in vars(parameters).items(): | ||||
if isinstance(value, (float, Constant)): | ||||
setattr(parameters, name, Function(R, val=float(value))) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
time_derivative, transport, prognostic, linearisation, | ||
pressure_gradient, coriolis, divergence, gravity, incompressible | ||
) | ||
from gusto.core.configuration import convert_parameters_to_real_space | ||
from gusto.equations.common_forms import ( | ||
advection_form, vector_invariant_form, | ||
kinetic_energy_form, advection_equation_circulation_form, | ||
|
@@ -105,6 +106,11 @@ def __init__(self, domain, parameters, | |
active_tracers=active_tracers) | ||
|
||
self.parameters = parameters | ||
# Convert the attributes of type ``float`` or ``firedrake.Constant`` | ||
# in the parameters to a function in real space. This conversion is a | ||
# preventive to avoid issues with adjoint computations, particularly | ||
# when the parameters are used as controls in sensitivity analyses. | ||
convert_parameters_to_real_space(parameters, domain.mesh) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if we did the conversion to reals initially, then we wouldn't need to do this in any of the equations? |
||
self.compressible = compressible | ||
|
||
w, phi, gamma = self.tests[0:3] | ||
|
@@ -168,10 +174,12 @@ def __init__(self, domain, parameters, | |
# -------------------------------------------------------------------- # | ||
if compressible: | ||
cs = parameters.cs | ||
# On assuming ``cs`` as a constant, it is right keep it out of the | ||
# integration. | ||
linear_div_form = divergence(subject( | ||
prognostic(cs**2 * phi * div(u_trial) * dx, 'p'), self.X)) | ||
prognostic(cs**2 * (phi * div(u_trial) * dx), 'p'), self.X)) | ||
divergence_form = divergence(linearisation( | ||
subject(prognostic(cs**2 * phi * div(u) * dx, 'p'), self.X), | ||
subject(prognostic(cs**2 * (phi * div(u) * dx), 'p'), self.X), | ||
linear_div_form)) | ||
else: | ||
# This enforces that div(u) = 0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ | |
) | ||
from gusto.equations.active_tracers import Phases, TracerVariableType | ||
from gusto.equations.prognostic_equations import PrognosticEquationSet | ||
|
||
from gusto.core.configuration import convert_parameters_to_real_space | ||
__all__ = ["CompressibleEulerEquations", "HydrostaticCompressibleEulerEquations"] | ||
|
||
|
||
|
@@ -45,7 +45,7 @@ def __init__(self, domain, parameters, sponge_options=None, | |
Args: | ||
domain (:class:`Domain`): the model's domain object, containing the | ||
mesh and the compatible function spaces. | ||
parameters (:class:`Configuration`, optional): an object containing | ||
x (:class:`Configuration`, optional): an object containing | ||
the model's physical parameters. | ||
sponge_options (:class:`SpongeLayerParameters`, optional): any | ||
parameters for applying a sponge layer to the upper boundary. | ||
|
@@ -101,6 +101,11 @@ def __init__(self, domain, parameters, sponge_options=None, | |
active_tracers=active_tracers) | ||
|
||
self.parameters = parameters | ||
# Convert the attributes of type ``float`` or ``firedrake.Constant`` | ||
# in the parameters to a function in real space. This conversion is a | ||
# preventive to avoid issues with adjoint computations, particularly | ||
# when the parameters are used as controls in sensitivity analyses. | ||
convert_parameters_to_real_space(parameters, domain.mesh) | ||
g = parameters.g | ||
cp = parameters.cp | ||
|
||
|
@@ -109,6 +114,7 @@ def __init__(self, domain, parameters, sponge_options=None, | |
u_trial = split(self.trials)[0] | ||
_, rho_bar, theta_bar = split(self.X_ref)[0:3] | ||
zero_expr = Constant(0.0)*theta | ||
# Check this for adjoints | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this is a leftover comment? |
||
exner = exner_pressure(parameters, rho, theta) | ||
n = FacetNormal(domain.mesh) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
import pytest | ||
import numpy as np | ||
|
||
from firedrake import * | ||
from firedrake.adjoint import * | ||
from pyadjoint import get_working_tape | ||
from gusto import * | ||
|
||
|
||
@pytest.fixture(autouse=True) | ||
def handle_taping(): | ||
yield | ||
tape = get_working_tape() | ||
tape.clear_tape() | ||
|
||
|
||
@pytest.fixture(autouse=True, scope="module") | ||
def handle_annotation(): | ||
from firedrake.adjoint import annotate_tape, continue_annotation | ||
if not annotate_tape(): | ||
continue_annotation() | ||
yield | ||
# Ensure annotation is paused when we finish. | ||
annotate = annotate_tape() | ||
if annotate: | ||
pause_annotation() | ||
|
||
|
||
@pytest.mark.parametrize("nu_is_control", [True, False]) | ||
def test_diffusion_sensitivity(nu_is_control, tmpdir): | ||
assert get_working_tape()._blocks == [] | ||
n = 30 | ||
mesh = PeriodicUnitSquareMesh(n, n) | ||
output = OutputParameters(dirname=str(tmpdir)) | ||
dt = 0.01 | ||
domain = Domain(mesh, 10*dt, family="BDM", degree=1) | ||
io = IO(domain, output) | ||
|
||
V = VectorFunctionSpace(mesh, "CG", 2) | ||
domain.spaces.add_space("vecCG", V) | ||
|
||
R = FunctionSpace(mesh, "R", 0) | ||
# We need to define nu as a function in order to have a control variable. | ||
nu = Function(R, val=0.0001) | ||
diffusion_params = DiffusionParameters(kappa=nu) | ||
eqn = DiffusionEquation(domain, V, "f", diffusion_parameters=diffusion_params) | ||
|
||
diffusion_scheme = BackwardEuler(domain) | ||
diffusion_methods = [CGDiffusion(eqn, "f", diffusion_params)] | ||
timestepper = Timestepper(eqn, diffusion_scheme, io, spatial_methods=diffusion_methods) | ||
|
||
x = SpatialCoordinate(mesh) | ||
fexpr = as_vector((sin(2*pi*x[0]), cos(2*pi*x[1]))) | ||
timestepper.fields("f").interpolate(fexpr) | ||
|
||
end = 0.1 | ||
timestepper.run(0., end) | ||
|
||
u = timestepper.fields("f") | ||
J = assemble(inner(u, u)*dx) | ||
|
||
if nu_is_control: | ||
control = Control(nu) | ||
h = Function(R, val=0.0001) # the direction of the perturbation | ||
else: | ||
control = Control(u) | ||
# the direction of the perturbation | ||
h = Function(V).interpolate(fexpr * np.random.rand()) | ||
|
||
# the functional as a pure function of nu | ||
Jhat = ReducedFunctional(J, control) | ||
|
||
if nu_is_control: | ||
assert np.allclose(J, Jhat(nu)) | ||
assert taylor_test(Jhat, nu, h) > 1.95 | ||
else: | ||
assert np.allclose(J, Jhat(u)) | ||
assert taylor_test(Jhat, u, h) > 1.95 |
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.
Should this actually just be
@property
?