diagnostics

[jshipton]

Each diagnostic field is a subclass of the DiagnosticField class and as such is required to have a name and a compute method. The diagnostic field classes are instantiated in the setup file and passed in to state. This causes several problems:

1. the diagnostic field may need to set up solvers using info from state which it won't have when it is instantiated
2. it is not easy to compose diagnostic fields - for example, the Sum class currently adds together 2 fields but it can't act on diagnostic fields because they are not in state.fields.

Suggested solutions (from @wence- ):

1. Pass the class constructor in to state, rather than an instance of the class. Then the diagnostic field class can have an init method if required, where solvers can be set up.
2. Make the compute method generic - i.e. it evaluates an expression. Then each diagnostic field class has an expression method that defines the symbolic expression for the diagnostic.

Discuss! @colinjcotter @hiroe- @dorugeber @thomasgibson @tommbendall (I'm tagging you but you don't have to care and conversely, if you're not tagged, I still want to hear from you if you do care!)

[jshipton]

My questions:

1. I don't understand what 'passing the class constructor' looks like.
2. in situations where you would like diagnosticA and diagnosticA + diagnosticB, is it possible to avoid calculating diagnosticA twice?

[wence-]

For 1:

The old diagnostics are like this:

class Foo(object):
    def __init__(self):
        pass

list_of_foos = [Foo(), Foo(), Foo()]
state = State(...., list_of_foos)

But really, every Foo wants to know about state, to make fields and the like.

So instead:

class Foo(object):
   def __init__(self, state):
       pass

list_of_foos = [Foo, Foo, Foo]
state = State(..., list_of_foos)

and State does:

class State(object):
    def __init__(self, list_of_foos):
         self.list_of_foos = [foo(self) for foo in list_of_foos]

[wence-]

Alternately, because this is sort of icky in its circular references, we could add a register_diagnostic_field method to State.

[wence-]

2. Not easily.

[jshipton]

How about if you're using the Sum diagnostic field, which requires you to tell it which fields to sum?

[wence-]

Maybe, we would need to think harder about how to design this.

[tommbendall]

An addition that I thought could be useful is to find the perturbation in a diagnostic variable (at the moment we only do it for prognostic variables?), which I thought I should mention because it seems vaguely relevant to Jemma's second problem.

[jshipton]

Yes @tommbendall, that's exactly the problem that we'd like to solve - I suppose you want a perturbation Exner pressure? We need to be able to define a diagnostic field that calculates the Exner pressure, a diagnostic field that calculates the background Exner pressure and have the Difference diagnostic cope with that without my current hack (which is on one of Hiroe's branches) of checking if the arguments are fields or diagnostic field class instances.

[tommbendall]

Yeah, I actually have a lot of hacked up diagnostic variables like that related to moisture somewhere on a branch! One thing I'd previously thought of is having a list of initial diagnostic fields that is a property of the diagnostics class, which could then be passed to other diagnostics, i.e.

class Diagnostics(object):

    def __init__(self, state):

        self.perturbation_fields = []

You'd also need an initialisation step though, like state.set_reference_profiles

[jshipton]

@tommbendall I'd be interested to see your hack-diagnostics-on-a-branch - could you point me to it?

[jshipton]

Maybe we need to postpone the setup of diagnostic fields until after state has been constructed - a bit like to do with initialisation (maybe this is what you meant @tommbendall ?) So state would need a setup_diagnostic_fields method which we would call from the setup file - then we could pass instances of the diagnostic field classes as we do now (so things like Sum would still work), and we would have state available so we can set up solvers in the init method.

I'm still not understanding how we can avoid something like the code in #104 lines 265-269 of diagnostics.py where we have to figure out if Sum needs to add 2 fields that are already in state.fields or whether it needs to get them from another diagnostic fields class. Would it help if diagnostic fields were added to state.fields? Then we would need some way of telling whether the diagnostic field had been calculated already that timestep before we tried to do anything with it.

I seem to have more questions than answers...

[wence-]

The problem is that a DiagnosticField doesn't behave like a Field (which is just a Function), or vice versa. So a DiagnosticField is just a Field, except that it is diagnosed in the time loop rather than prognosed. So potentially having all fields in state.fields (and maybe splitting that internally into prognostic_fields and diagnostic_fields[and prescribed_fields]) would be better. Then you can always ask for a field by saying state.fields(name) and one can do the necessary magic to make sure that diagnostic fields are computed at that point, I guess.

[tommbendall]

The stuff I'd done is in diagnostics.py on the moist_hydrostatic branch. I wouldn't recommend doing what I did though -- in my test cases I was setting up new fields (e.g. initial Exner pressure). These initial fields don't do anything except get read into diagnostics.

[jshipton]

OK, here's my train of thought and possible solution...

• I think DiagnosticField classes need to be instantiated in the setup file - because otherwise, for example, you can't tell Sum which 2 fields to sum.
• The field method of the DiagnosticField class is there because 1) we need a place for the class writer to tell the code which function space to create the field on, and 2) we don't have the mesh info until after we've set up state.
• It would be easier if the diagnostic fields were stored in state.fields.

So, how about we replace the field method in the DiagnosticField class with a function_space property. Also, as @wence- suggested above, we make compute a generic method, that always evaluates expression which is a method defined in each DiagnosticField class. Then, in state.py, after we have instantiated FieldCreator (which creates state.fields) we add each diagnostic field to state.fields:

for diagnostic in diagnostic_fields:
   self.fields(diagnostic.function_space, expr=diagnostic.expression, calculated=False)

where expr and calculated are treated like the pickup and dump tags (see line 48 of state.py). Then when we compute a diagnostic field, we can check whether the fields it depends on have an expression that needs to be evaluated and whether they have been calculated already (the idea being that the calculated tag is set to True once that field has been calculated and reset to False at the beginning of each timestep). We could even have a recalculate tag that will be False for things like the background Exner pressure which only needs calculating once at the beginning of the simulation but is required to calculate the perturbation Exner pressure at each timestep.

I think something like this could be made to work, but I'm not sure if people think it is good code practise or not. Thoughts?

[jshipton]

One possible problem with a generic compute method is that we can't assume that we can always interpolate an expression - maybe use

try:
   self.field.interpolate(expr)
except WhateverError
   self.field.project(expr)

? Can we ever just assign and what difference does it make if we interpolate instead?

[tommbendall]

I have a proposal that's relevant to some of the issues raised here.

I've made a subclass of Diagnostic called Perturbed_Diagnostic. This takes a diagnostic field as an argument, and has a new initialise method. The initial diagnostic field is then an attribute of the instance of Perturbed_Diagnostic, and it is initialised during the setup_dump method of state.

I think this allows us to view perturbed diagnostic fields in an easy way. I'm not sure it solves either of the problems Jemma originally raised however...

If we're looking to do some major rewriting of diagnostics.py then this probably isn't very helpful, but if this otherwise seems sensible then I could put in a pull request?

[jshipton]

What branch is that on Tom? I'm still interested in making this all a bit tidier...

[tommbendall]

I'm afraid it's not on its own branch at the moment! It's actually on the moist_hydrostatic branch.

I actually added another feature yesterday. I can now do two things: make the base diagnostic state from the initial prognostic variables, or you can choose your own base diagnostic state.

It's not very neat but it allowed me to do both of those things easily...

[colinjcotter]

Hi Tom, We should talk about getting your stuff into master in incremental chunks otherwise you are going to drift further and further away from what we have! cheers --cjc

…

On 11 July 2017 at 19:19, tommbendall ***@***.***> wrote: I'm afraid it's not on its own branch at the moment! It's actually on the moist_hydrostatic branch. I actually added another feature yesterday. I can now do two things: make the base diagnostic state from the initial prognostic variables, or you can choose your own base diagnostic state. It's not very neat but it allowed me to do both of those things easily... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#106 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF0q8szLkOJ2swydJFol-St0QrUh5_ONks5sM66qgaJpZM4NwPoy> .

-- http://www.imperial.ac.uk/people/colin.cotter www.cambridge.org/9781107663916

[tommbendall]

I know! I don't think it should be much of a problem, I was going to put in pull requests in stages after the Gung Ho meeting next week.