Esmvaltool: Better diagnostic organization

[bouweandela]

Currently all diagnostics are in a directory esmvaltool/diag_scripts and bits of NCL code are scattered throughout the Python library. It would be good to take diag_scripts folder out of the esmvaltool python package and add some organization, with a standard location to add library functions. I discussed with Niels and we came up with the following proposal for a safer and cleaner directory structure.

Example:

esmdiagnostics/
├── ncl
│   ├── my_diag
│   │   ├── test_pr.ncl
│   │   ├── test_ta.ncl
│   │   └── test_ta_no2.ncl
│   ├── perfmetrics
│   │   ├── grading_collect.ncl
│   │   ├── grading.ncl
│   │   ├── main.ncl
│   │   ├── taylor_collect.ncl
│   │   └── taylor.ncl
│   └── shared
│       └── aux_perfmetrics.ncl
├── python
│   ├── my_diag
│   │   ├── test_pr.py
│   │   ├── test_ta_no2.py
│   │   └── test_ta.py
│   ├── shared
│   │   ├── esmdiaglib.py
│   │   └── mylib
│   │       └── __init__.py
│   └── some_other_diag
│       ├── test_pr.py
│       ├── test_ta_no2.py
│       └── test_ta.py
└── r
    ├── shared
    └── some_r_diagnostic
        └── xxx.r

The idea is to create a single directory esmdiagnostics, which contains all diagnostics and related code organized in a single directory per language. Every diagnostic will live in it's own directory and per language there is one directory shared in which all shared code is placed. Diagnostic developers can then create a file (or a subdirectory containing a collection of related files) in the shared directory if something is shared between multiple diagnostics.

When running a diagnostic from ESMValTool via a namelist, the path of the diagnostic itself will be available in an environmental variable ESM_DIAG_PATH. The path of the shared directory of the language that the diagnostic is written in, will be available in the environmental variable ESM_DIAG_SHARED_PATH, e.g. for an NCL diagnostic ESM_DIAG_SHARED_PATH=/esmdiagostics/ncl/shared.

NCL diagnostics
In NCL, load paths starting with ./ can thus be eliminated in favour of paths starting with either "$ESM_DIAG_PATH" or "$ESM_DIAG_SHARED_PATH", to avoid having to set the current working directory to the location of the NCL diagnostic scripts when executing.

Python diagnostics
ESM_DIAG_SHARED_PATH will also be appended to PYTHONPATH, so in the example above it is possible to do import mylib in the python diagnostic scripts, e.g. in test_pr.py. A library function esmdiaglib.get_diag_path() will be made available that returns the value of ESM_DIAG_PATH to avoid having to use the environmental variables directly from Python in diagnostic scripts. Python functions from the esmvaltool module can also be used in Python diagnostics by using import esmvaltool after installation of the package.

All diagnostics should be executable scripts that take no command line arguments.

Preferably all paths should be lower case only.

[bouweandela]

Example with more content:

├── ncl
│   ├── my_diag
│   │   ├── test_pr.ncl
│   │   ├── test_ta.ncl
│   │   └── test_ta_no2.ncl
│   ├── perfmetrics
│   │   ├── grading_collect.ncl
│   │   ├── grading.ncl
│   │   ├── main.ncl
│   │   ├── taylor_collect.ncl
│   │   └── taylor.ncl
│   ├── shared
│   │   ├── aux_perfmetrics.ncl
│   │   ├── ensemble.ncl
│   │   ├── latlon.ncl
│   │   ├── legacy_code
│   │   │   ├── ensemble.ncl
│   │   │   └── statistics.ncl
│   │   ├── misc_function.ncl
│   │   ├── regridding.ncl
│   │   ├── rgb
│   │   │   ├── amwg.rgb
│   │   │   ├── eyring_toz.rgb
│   │   │   ├── ipcc-od550aer-delta.rgb
│   │   │   ├── ipcc-od550aer.rgb
│   │   │   ├── ipcc-precip-delta.rgb
│   │   │   ├── ipcc-precip.rgb
│   │   │   ├── ipcc-tas-delta.rgb
│   │   │   ├── ipcc-tas.rgb
│   │   │   ├── qcm3.rgb
│   │   │   ├── rainbow.rgb
│   │   │   └── red-blue.rgb
│   │   ├── scaling.ncl
│   │   ├── set_operators.ncl
│   │   ├── statistics.ncl
│   │   ├── style.ncl
│   │   └── styles
│   │       ├── CCMVal1.style
│   │       ├── CCMVal2.style
│   │       ├── CMIP5.style
│   │       ├── DEFAULT.style
│   │       └── righi15gmd.style
│   └── WAMonsoon
│       ├── 10W10E_1D_basic.ncl
│       ├── 10W10E_3D_basic.ncl
│       ├── autocorr.ncl
│       ├── cmap_difference_pres.rgb
│       ├── cmap_difference.rgb
│       ├── cmap_difference_tas.rgb
│       ├── cmap_difference_theta.rgb
│       ├── cmap_difference_wind.rgb
│       ├── contour_basic.ncl
│       ├── exact_panel_positions_pres.ncl
│       ├── exact_panel_positions_pr-mmday.ncl
│       ├── exact_panel_positions_tas.ncl
│       ├── exact_panel_positions_wind.ncl
│       ├── isv_filtered.ncl
│       ├── precip_IAV.ncl
│       ├── precip_seasonal.ncl
│       └── wind_basic.ncl
├── python
│   ├── my_diag
│   │   ├── test_pr.py
│   │   ├── test_ta_no2.py
│   │   └── test_ta.py
│   ├── shared
│   │   ├── esmdiaglib.py
│   │   └── mylib
│   │       └── __init__.py
│   ├── sm_pr_diag
│   │   ├── global_rain_sm.f90
│   │   ├── sample_events.f90
│   │   ├── sm_pr_diag_nml.py
│   │   └── sm_pr_diag_test.py
│   └── some_other_diag
│       ├── test_pr.py
│       ├── test_ta_no2.py
│       └── test_ta.py
└── r
    ├── shared
    └── some_r_diagnostic
        └── xxx.r

[BenMGeo]

This would be fine, if none overlap exists, e.g. a ncl script running a py script to calculate some subroutines. I support this idea.

[bouweandela]

Another possibility would be the following. This would make it easier for users to find the diagnostic they are looking for, because diagnostics are no longer organized per language, which also allows using multiple languages related to a single diagnostic theme. This solution also addresses the issue raised by @BenMGeo .

esmdiagnostics/
├── my_diag
│   ├── test_pr.py
│   ├── test_ta_no2.py
│   └── test_ta.py
├── my_ncl_diag
│   ├── test_pr.ncl
│   ├── test_ta.ncl
│   └── test_ta_no2.ncl
├── perfmetrics
│   ├── grading_collect.ncl
│   ├── grading.ncl
│   ├── main.ncl
│   ├── taylor_collect.ncl
│   └── taylor.ncl
├── shared
│   ├── ncl
│   │   ├── aux_perfmetrics.ncl
│   │   ├── ensemble.ncl
│   │   ├── latlon.ncl
│   │   ├── legacy_code
│   │   │   ├── ensemble.ncl
│   │   │   └── statistics.ncl
│   │   ├── misc_function.ncl
│   │   ├── plot
│   │   │   ├── aux_plotting.ncl
│   │   │   ├── contour_maps.ncl
│   │   │   ├── contourplot.ncl
│   │   │   ├── GO_panels.ncl
│   │   │   ├── legacy_code
│   │   │   │   └── aux_plotting.ncl
│   │   │   ├── legends.ncl
│   │   │   ├── mjo_level1.ncl
│   │   │   ├── mjo_level2.ncl
│   │   │   ├── monsoon_domain_panels.ncl
│   │   │   ├── monsoon_panels.ncl
│   │   │   ├── portrait_plot.ncl
│   │   │   ├── rgb
│   │   │   │   ├── amwg.rgb
│   │   │   │   ├── eyring_toz.rgb
│   │   │   │   ├── ipcc-od550aer-delta.rgb
│   │   │   │   ├── ipcc-od550aer.rgb
│   │   │   │   ├── ipcc-precip-delta.rgb
│   │   │   │   ├── ipcc-precip.rgb
│   │   │   │   ├── ipcc-tas-delta.rgb
│   │   │   │   ├── ipcc-tas.rgb
│   │   │   │   ├── qcm3.rgb
│   │   │   │   ├── rainbow.rgb
│   │   │   │   └── red-blue.rgb
│   │   │   ├── scatterplot.ncl
│   │   │   ├── styles
│   │   │   │   ├── CCMVal1.style
│   │   │   │   ├── CCMVal2.style
│   │   │   │   ├── CMIP5.style
│   │   │   │   ├── DEFAULT.style
│   │   │   │   └── righi15gmd.style
│   │   │   ├── taylor_diagram_less_hardcoded.ncl
│   │   │   ├── taylor_plot.ncl
│   │   │   ├── vector_scalar_map_polar.ncl
│   │   │   ├── xy_line.ncl
│   │   │   └── zonalmean_profile.ncl
│   │   ├── regridding.ncl
│   │   ├── scaling.ncl
│   │   ├── set_operators.ncl
│   │   ├── statistics.ncl
│   │   └── style.ncl
│   ├── python
│   │   ├── esmdiaglib.py
│   │   └── mylib
│   │       └── __init__.py
│   └── r
├── sm_pr_diag
│   ├── global_rain_sm.f90
│   ├── sample_events.f90
│   ├── sm_pr_diag_nml.py
│   └── sm_pr_diag_test.py
├── some_other_diag
│   ├── test_pr.py
│   ├── test_ta_no2.py
│   └── test_ta.py
├── some_r_diagnostic
│   └── xxx.r
└── WAMonsoon
    ├── 10W10E_1D_basic.ncl
    ├── 10W10E_3D_basic.ncl
    ├── autocorr.ncl
    ├── cmap_difference_pres.rgb
    ├── cmap_difference.rgb
    ├── cmap_difference_tas.rgb
    ├── cmap_difference_theta.rgb
    ├── cmap_difference_wind.rgb
    ├── contour_basic.ncl
    ├── exact_panel_positions_pres.ncl
    ├── exact_panel_positions_pr-mmday.ncl
    ├── exact_panel_positions_tas.ncl
    ├── exact_panel_positions_wind.ncl
    ├── isv_filtered.ncl
    ├── precip_IAV.ncl
    ├── precip_seasonal.ncl
    └── wind_basic.ncl

[axel-lauer]

In principle, a better organization of the diagnostic scripts would be nice. The proposed ideas sound good for the easy cases, but I guess things become a bit more complicated for some other cases. Some code of the SAMonsoon diagnostic, for instance, is also used by the diagnostics DiurnalCycle, Evapotranspiration, Global Ocean, IPCC Ch. 9, and WAMonsoon, which might make sorting the diagnostics in the proposed way difficult if not confusing. I have no better idea yet, but I guess this might need some more fine tuning...

[bouweandela]

@axel-lauer The case you describe above seems to be covered by the proposed directory structure.

The WAMonsoon code that is shared with other diagnostics could for example go into
esmdiagnostics/shared/ncl/monsoon
or if it is code for plotting it could go into
esmdiagnostics/shared/ncl/plot
This would make it easier to see that this code is shared.

[bouweandela]

Maybe it would be better to also get rid of the language specific subdirectories in the shared directory and instead create a subdirectory for scripts that are related for some reason, e.g. like this:
esmdiagnostics/shared/monsoon
esmdiagnostics/shared/plot
esmdiagnostics/shared/statistics
This would make it easier to find tools related to a particular task, plus a single set of tools might use multiple languages.

[mattiarighi]

We discussed this internally.

We find the idea good in principle but we are also afraid that it could generate a lot of issues (see above comment by Axel). Since our main focus now is to get the new backend and interface running, we would propose to postpone it.

Once the backend is finalized, we plan to have a coding workshop where we will focus on testing all namelist and diagnostics in the new version. That could be the opportunity to discuss this issue again, but for the moment we shall focus on the other issues which are more urgent.

@ESMValGroup/esmvaltool-coreteam

[BenMGeo]

I fully second this.

Ben

On 12.12.2017 13:20, Mattia Righi wrote:
>

We discussed this internally.

We find the idea good in principle but we are also afraid that it
could generate a lot of issues (see above comment by Axel). Since our
main focus now is to get the new backend and interface running, we
would propose to postpone it.

Once the backend is finalized, we plan to have a coding workshop where
we will focus on testing all namelist and diagnostics in the new
version. That could be the opportunity to discuss this issue again,
but for the moment we shall focus on the other issues which are more
urgent.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ESMValGroup/ESMValTool/issues/106#issuecomment-351035774,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AOSImkL3nuy7mXDwCrdAUCxo9mo6uz1Jks5s_m-dgaJpZM4PwafY.

--
M. Sc.
Scientific Programmer
Department of Geography
LMU - Ludwig-Maximilians-Universitaet

Luisenstraße 37
80333 München

Tel.: [phone]
E-Mail: B.[email protected]

[bouweandela]

I think this issue needs to be addressed before starting to port the diagnostics from version 1.x to version 2 of esmvaltool, as it will be much harder to change this afterwards.

[mattiarighi]

As discussed above, we shall avoid too complicated nestings.
My suggestion would be to simply sort diagnostics by language (as in plots_scripts):

diag_scripts\ncl\*.ncl
diag_scripts\ncl\lib\*.ncl
diag_scripts\python\*.py
diag_scripts\python\lib\*.py
diag_scripts\R\*.r
diag_scripts\R\lib\*.r

As long as we port the diag script to v2, we might also think about introducing a naming convention for the scripts (e.g., avoid upper-case, see #12).

@axel-lauer

[bouweandela]

One of the concerns raised at the sprint at KNMI last week was that we may end up writing the same functionality in many different languages (Python, NCL, R, ..). Therefore I would recommend against creating language specific directories, as this makes it harder for diagnostic developers to find functionality that is already implemented.

With the new namelist format, it is quite easy to use functionality implemented in a different language: a single diagnostic section in the namelist can use diagnostic scripts in multiple languages, the ancestors keyword can be used to run multiple diagnostic scripts one after the other, while the output of one or more scripts is used as the input of the next script. See e.g. the perfmetrics namelist for an example of this.

There seemed to be some agreement on the structure I proposed on October 9. I would propose to go for that, but drop the language specific directories. It does impose some grouping of diagnostic scripts, but not much:

diag_scripts/
├── my_diag
│   ├── test_pr.py
│   ├── test_ta_no2.py
│   └── test_ta.py
├── my_ncl_diag
│   ├── test_pr.ncl
│   ├── test_ta.ncl
│   └── test_ta_no2.ncl
├── perfmetrics
│   ├── grading_collect.ncl
│   ├── grading.ncl
│   ├── main.ncl
│   ├── taylor_collect.ncl
│   └── taylor.ncl
├── shared
│   ├── aux_perfmetrics.ncl
│   ├── ensemble.ncl
│   ├── latlon.ncl
│   ├── legacy_code
│   │   ├── ensemble.ncl
│   │   └── statistics.ncl
│   ├── misc_function.ncl
│   ├── plot
│   │   ├── aux_plotting.ncl
│   │   ├── contour_maps.ncl
│   │   ├── contourplot.ncl
│   │   ├── GO_panels.ncl
│   │   ├── legacy_code
│   │   │   └── aux_plotting.ncl
│   │   ├── legends.ncl
│   │   ├── mjo_level1.ncl
│   │   ├── mjo_level2.ncl
│   │   ├── monsoon_domain_panels.ncl
│   │   ├── monsoon_panels.ncl
│   │   ├── portrait_plot.ncl
│   │   ├── rgb
│   │   │   ├── amwg.rgb
│   │   │   ├── eyring_toz.rgb
│   │   │   ├── ipcc-od550aer-delta.rgb
│   │   │   ├── ipcc-od550aer.rgb
│   │   │   ├── ipcc-precip-delta.rgb
│   │   │   ├── ipcc-precip.rgb
│   │   │   ├── ipcc-tas-delta.rgb
│   │   │   ├── ipcc-tas.rgb
│   │   │   ├── qcm3.rgb
│   │   │   ├── rainbow.rgb
│   │   │   └── red-blue.rgb
│   │   ├── scatterplot.ncl
│   │   ├── styles
│   │   │   ├── CCMVal1.style
│   │   │   ├── CCMVal2.style
│   │   │   ├── CMIP5.style
│   │   │   ├── DEFAULT.style
│   │   │   └── righi15gmd.style
│   │   ├── taylor_diagram_less_hardcoded.ncl
│   │   ├── taylor_plot.ncl
│   │   ├── vector_scalar_map_polar.ncl
│   │   ├── xy_line.ncl
│   │   └── zonalmean_profile.ncl
│   ├── regridding.ncl
│   ├── scaling.ncl
│   ├── set_operators.ncl
│   ├── statistics.ncl
│   └── style.ncl
│   ├── esmdiaglib.py
│   └── mylib
│       └── __init__.py
├── sm_pr_diag
│   ├── global_rain_sm.f90
│   ├── sample_events.f90
│   ├── sm_pr_diag_nml.py
│   └── sm_pr_diag_test.py
├── some_other_diag
│   ├── test_pr.py
│   ├── test_ta_no2.py
│   └── test_ta.py
├── some_r_diagnostic
│   └── xxx.r
└── WAMonsoon
    ├── 10W10E_1D_basic.ncl
    ├── 10W10E_3D_basic.ncl
    ├── autocorr.ncl
    ├── cmap_difference_pres.rgb
    ├── cmap_difference.rgb
    ├── cmap_difference_tas.rgb
    ├── cmap_difference_theta.rgb
    ├── cmap_difference_wind.rgb
    ├── contour_basic.ncl
    ├── exact_panel_positions_pres.ncl
    ├── exact_panel_positions_pr-mmday.ncl
    ├── exact_panel_positions_tas.ncl
    ├── exact_panel_positions_wind.ncl
    ├── isv_filtered.ncl
    ├── precip_IAV.ncl
    ├── precip_seasonal.ncl
    └── wind_basic.ncl

Basically this structure is as simple as: if all required functionality for a diagnostic is implemented in a single file, create a file, if it is implemented in multiple files, put them together in a directory. If the functionality in a diagnostic script is shared across multiple diagnostic scripts, move those functions to the shared directory, again either as a single file or as a set of files in a directory. This will facilitate easy discovery of available functionality and helps to keep track of what is shared by multiple diagnostics (this also helps to discover what is already implemented, as the shared code is much more likely to be relevant for new diagnostic development than what is available in other diagnostic scripts).

I had a discussion with @nielsdrost and we both thought that shared was a slightly clearer name than lib for the location where code shared between multiple diagnostics is stored, but if this is an issue for you we can keep lib of course.

[mattiarighi]

Let me discuss this again with @axel-lauer et al. and then take a decision.
As you said above, we need to come up with a structure, before start including other diags.

[axel-lauer]

I am fine with the last proposed option including no language specific folders and moving code from "lib" and "aux" to "shared".

[mattiarighi]

Working on this issue here.