Redesign of workflow and objects

[jsmariegaard]

After explaining to users how to use fmskill and observing how they use it, it has become clear that some parts of the workflow are not optimal - especially that you add_observation() on ModelResult. That is not natural and is a source of confusion.

One way to improve the structure and workflow is to add a new object Connector between Observation and ModelResult which connects the two (instead of "adding" one to the other). I suggest also making Observation and ModelResult a "Mapping" such that when you define one you don't need to specify an item.

mr1 = ModelResult("HKZN_local_2017_DutchCoast.dfsu", name='SW_1')
mr1["Significant wave height"]    # this is a ModelResultItem

o1 = PointObservation("HKNA_Hm0.dfs0", x=4.2420, y=52.6887, name="HKNA")
o1[0]    # this is a PointObservationItem

con = Connector(o1[0], mr1["Significant wave height"])    # does validation
cc = con.extract()

For simple comparisons we could also have a function compare, that directly returns the comparison:

 cc = fmskill.compare(o1['surface elevation'], [mr1[0], mr2[0]])

Also for multiple models and/or multiple observations. The function compare() will do two things: connect and extract. Optionally, the extract step could be lazy, such that the actual extraction is not done before the comparison is needed.

Gains:

• It will become easier to understand what goes on (simpler to onboard new users)
• Code will be simpler - more natural split

Potential issues/difficulties with the new approach:

• more difficult to achieve "smart" extraction (important for large dfsu)
• breaking changes
• could be difficult to understand yet another object for some
• specification of Connection: validate, interpolation_type etc
• specification of custom variable names should be done on a xItem level
• specification of custom itemInfo in PointObservation

Comments:

• It should be possible to construct both ModelResult and Observation with a specific item only (and a start/end)
• It should be possible to call compare() and extract() with a start/end argument
• If a ModelResult or Observation only has one item it should not be necessary to pass to connect() or compare() with [0]
• Would it be better to have an object Connector on which you call Connector.add(), which holds all the connections until you call extract()
• Alternative terms for the Connector/connection could be: Match, Associator, Relation, Link, Joint, Junction

[Hendrik1987]

I think it's a good option to introduce the Connector Especially the multi model comparison will become easier for the user to follow. Creating a CompareCollection from the things you want to compare is very intuitive.

• smart extraction
  Is it harder to achieve 'smart' extraction? ModelResult and Observations don't have to load any data into memory, all they to is 'link' the files and get information on temporal and spatial coverage. By the time the extraction is done in Connection all model results are specified - or what am I missing here?

• lazy extraction
  I think is a very good idea. Is this hard to achieve in an efficient way? What if the queried data keeps changing? Say you do cc.skill(area=A1) followed by cc.skill(area=A2) where A1 and A2 are overlapping. Currently both call metrics on different views of the df, all handled by pandas, done. For lazy extraction, would you first create a df only containing the data within A1 and then find the missing data for A2 and add it to the df? Would you drop the data that is in A2 but not A1 again? We can possibly learn from xarray (at least I would have to).

• adding model results later
  Say you do a calibration. You typically start with a handful of models, assess them, run some more and add them to your comparison later. I guess cc = cc1 + cc2 would still work, right? Is this the best way? Also see next point.

• big data / memory / structure of df
  For the use case of comparing a global model to large amounts of data, I realized that memory could actually become an issue.
  One workaround is to go through the workflow model by model, and to only concat the skill dataframes at the very end to have a nice overall overview. Maybe that is fine, as for most of the use cases we don't run into any memory issues.
  Else, we would have to revisit the way of how we store data in the ComparerCollection. The 'one big dataframe' is convenient, but comes with a lot of data duplicates. Changing this is likely not justified, but maybe we should think about it once again at his early stage.

• the name...
  Link is nice. Or Map maybe (also indicating that you actually change the time and space reference of the model results or 'left side')? In any way, I prefer short :-)

As a bonus?

• Easy option for additional functionality: Model vs Model and Observation vs Observation?
  For the main purpose of fmskill, (multiple) model results are mapped to observation space and then compared. Additional functionality might easily be added through the Connector.

Connection([m1,m2,...], [o1,o2,...]) # base case
Connection(o1, o2)  # quick comparison of o1 to o2
Connection([m1,m2,m3], m4, points = [{...}]) # compare multiple models to base case model, some definition of location is required

Yes, the additional two are not a skill assessment but at least for me a very common task - so it would be awesome if this capacity was shipped with fmskill :-)

[jsmariegaard]

Thanks a lot for all you thoughts, @Hendrik1987 ! Much appreciated.

[ecomodeller]

I am wondering if this discussion is outdated?

[jsmariegaard]

I am wondering if this discussion is outdated?

yeees? I guess that is quite obvious from the dates...

[jsmariegaard]

The quick-and-dirty comparison was handled by #55

The major revision and introduction of the new Connector object is now ongoing in #56