-
Notifications
You must be signed in to change notification settings - Fork 13
Enable adjoint #607
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
Changes from 34 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
998deb1
ae31058
39829b3
ed8b8b8
4971e1b
5b16837
6585dfd
ea044e6
1e32799
779756c
4cbdadb
f973f84
d9065e3
74762da
837ec30
65e274b
f5ed0dc
c5060de
93e06d6
9c5a13f
d6c1287
e906cd6
1ec4322
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): | ||||
|
Contributor
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
Contributor
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
Constants? Can we just do straight to real functions?
Contributor
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
Contributor
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
Contributor
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!
Contributor
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) | ||
|
Contributor
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 | ||
|
Contributor
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?