mcgibbon / sympl

A toolkit for building planetary/Earth system models in Python
http://sympl.readthedocs.io
Other
50 stars 14 forks source link

Rename Diagnostic #32

Closed mcgibbon closed 6 years ago

mcgibbon commented 6 years ago

Some people have brought up and we have also found that it's a little confusing to talk about Diagnostics producing diagnostics, especially when Prognostics produce diagnostics along with every other component. I'm not sure what a better name is, however, and we've been pondering this for some time. It may be that we need to rename multiple components so they all have a similar style of naming.

Some possibilities:

JoyMonteiro commented 6 years ago

I don't understand what Scheme means here. Are you suggesting we extend the base class names by adding suffixes (and create subclasses) or create a large number of new classes?

I think the confusion arises from the fact that there exists no model currently with a set of classes like sympl does. diagnostics are part of the output, and no one cares how they are generated. I'm hesitant to accommodate a change which takes sympl backwards in time to make it correspond well with current model structure/usage.

mcgibbon commented 6 years ago

I mean extend the base class names, without creating any subclasses or new classes. Just changing the names of the base classes.

It's confusing to have the word "diagnostic" refer to either a Diagnostic component or a diagnostic quantity. This is confusing regardless of the current modeling paradigm.

JoyMonteiro commented 6 years ago

In that case, I would prefer adding the Component suffix -- makes it very clear what it is.

mcgibbon commented 6 years ago

I agree that this appears to be the cleanest option at the moment. I'll do this for 0.4.0 so we can use this language in the paper, along with the Implicit -> Stepper change.

mcgibbon commented 6 years ago

Final language change is Diagnostic --> DiagnosticComponent Prognostic --> TendencyComponent Implicit --> Stepper TimeStepper --> TendencyStepper

Additionally, TimeStepper will be a subclass of Stepper.

JoyMonteiro commented 6 years ago

Ack. Close this issue now?

mcgibbon commented 6 years ago

I'll close this and several other issues when we merge their pull requests.

mcgibbon commented 6 years ago

Fixed by #26