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

Set correct initial conditions for fields excluded from calibration #153

Closed
adelinehillier opened this issue Jan 31, 2022 · 8 comments
Closed

Comments

@adelinehillier
Copy link
Collaborator

We need a way to be able to set initial conditions for fields that are not to be included in the calibration. Currently, if we want to calibrate the purely convective simulation considering only the field b and we start the calibration from 3 hours, fields u, v, and e will be set to all zeros because they have not been stored in the observation. It seems to me that we should automatically store all available fields at all time steps in observations and:

(1) remove field_names from the arguments of SyntheticObservations and provide some kind of Exclude normalization option that will remove the field from the forward map output altogether, or automatically exclude all fields whose normalizations have not been specified.
(2) add a field_names attribute to InverseProblem that keeps track of, for each observation, which fields are to be included in the output map (and therefore which fields are to be tracked in the time_series_collector).
(3) add a ‘field_namesattribute toSyntheticObservations` that keeps track of which fields are to be included in the output map for that observation.

I think option (1) makes the most sense since the choice of field_names directly pertains to the output map. Thoughts?

@glwagner
Copy link
Member

glwagner commented Jan 31, 2022

There's another solution that doesn't require any changes to the API: we can add a property (either to SyntheticObservations or InverseProblem called initialize!, which is a function of simulation that runs in initialize_simulation!, perhaps after set!(simulation.model, observations, time_index)

https://github.com/CliMA/OceanTurbulenceParameterEstimation.jl/blob/57e4b097d95803c5d27c42286c6fcbb4ded0dd06/src/Observations.jl#L294-L295

Loading additional or "auxiliary" fields into the Observations is a slightly different issue under the above solution, but auxiliary field loading could be a nice convenience feature for initialize! . Another solution is a property auxiliary_field_time_serieses for SyntheticObservations. But perhaps an even better solution is to develop Oceananigans' FieldDataset:

https://github.com/CliMA/Oceananigans.jl/blob/main/src/OutputReaders/field_dataset.jl

so that users can easily build initialization functions... ?

Something to consider is that there may be initialization requirements that can't be expressed in a Field. We've managed to consider problems simple enough so that we don't have that requirement, but I'm not sure we'll always have that. Inserting a user-defined function into initialize_simulation permits general behavior which might be nice.

I think "multipurposing" objects (eg using field_time_serieses to hold fields both for convenience initialization, and for loss calculation) adds complexity that's sometimes better to avoid.

@glwagner
Copy link
Member

glwagner commented Jan 31, 2022

The Exclude feature can be achieved by using RescaledZScore(0), right?

@navidcy
Copy link
Collaborator

navidcy commented Feb 6, 2022

Yeah! I might be hitting the point where I need this. :)

@glwagner
Copy link
Member

glwagner commented Feb 6, 2022

I'll work on this today.

@glwagner
Copy link
Member

glwagner commented Feb 10, 2022

It might be simple support excluding fields now using Transformation! We just need a special type (at worst we'll have to do something silly like transform a field to zeros(1, 1, 1, 1)) but something cleaner might be doable too...

I think we'll still need InverseProblem.initialize_simulation! too.

@glwagner
Copy link
Member

glwagner commented Feb 21, 2022

@navidcy and I implemented this feature for SyntheticObservations by adding a property forward_map_names. Rather than "excluding" fields, we provide two lists of fields: one to be used for the forward map, and one to be loaded for the purpose of setting initial conditions or plotting later (we only initialize fields(simulation.model), so it's also possible to load fields into SyntheticObservations that are part of the forward map, but are not part of the initialization).

@navidcy I don't see this feature on main yet --- is it on your baroclinic adj calibration PR? I'd like to spend some time on docs this week so it'd be good to get that in!

@navidcy
Copy link
Collaborator

navidcy commented Feb 21, 2022

I opened #217 but I'm debating whether I should cherry-pick the relevant files and leave the calibration attempts out of it. :)

@glwagner
Copy link
Member

ok!

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

No branches or pull requests

3 participants