Ouranosinc / xscen

A climate change scenario-building analysis framework.
https://xscen.readthedocs.io/
Apache License 2.0
17 stars 2 forks source link

Better documentation for the templates #232

Closed RondeauG closed 1 year ago

RondeauG commented 1 year ago

Generic Issue

Description

I've been helping @vindelico get accustomed to xscen in the last few weeks, and he had a few suggestions for our templates (especially the first 1, which is the most complete one).

  1. Be as exhaustive as possible. In some places in the config, we only write the arguments that are being overwritten. The reason why some arguments are missing wasn't clear. Something like this would be preferable:

    climatological_mean: # automatically passed to the function
    window: 30
    min_periods:  # Default value, so you would not need to include it in a real configuration file.
    interval: 10
    periods: [['1951', '2100']]
    to_level: climatology
  2. Better explain why certain functions (i.e. compute_deltas) are under specific categories (i.e aggregate)

    # Functions under this section are associated to xscen/aggregate.py
    aggregate:
    dask:
    n_workers: 4
    threads_per_worker: 4
    memory_limit: "6GB"
    ...
  3. Better order/indicate which function gets sent automatically to xscen through parse_config and which doesn't.

    
    aggregate:
    # These entries do not correspond to an `xscen` function, but if you plan to have multiple variations of them
    # (i.e. to fine-tune dask workers depending on the task to perform), then categorizing them under `aggregate` 
    # makes it easier to locate them within the CONFIG object when you want to perform tasks related to the
    # `aggregate` module, such as when computing `climatological_mean`
    dask:
    n_workers: 4
    threads_per_worker: 4
    memory_limit: "6GB"
    input:
    clim:
      processing_level: indicators
    delta:
      processing_level: climatology
    save:
    mode: 'o'
    
    # These entries exactly match `xscen` functions located under the `aggregate` module, so the parameters 
    # specified below will automatically override default values. 
    # See https://xscen.readthedocs.io/en/latest/notebooks/6_config.html for more details on how it works.
    climatological_mean: 
    window: 30
    interval: 10
    periods: [['1951', '2100']]
    to_level: climatology
    compute_deltas: 
    kind: "+"
    reference_horizon: "1991-2020"
    to_level: 'delta'
juliettelavoie commented 1 year ago

I can help with this.

juliettelavoie commented 1 year ago
  1. I am not convinced this is a good idea. It would make the config wayyy longuer and harder to read. The default being used when not passing an argument is not a weird behavior.
  2. All the functions that were already marked by "# automatically passed to the function". I am adding some text at the beginning to clarify the difference btw automatically passed and manually.
vindelico commented 1 year ago
  • I am not convinced this is a good idea. It would make the config wayyy longuer and harder to read.

Why not use this file for exhaustive instructions?

I started out with this config to use xscen and you have to work with it a lot to get the workflow to run. So I went ahead and inserted what I think would be helpful for newcomers. I would also suggest (as commented) to put similar exhaustive comments in the first task of the workflow.

The default being used when not passing an argument is not a weird behavior.

I agree. However, it would be useful to showcase where this 'invisible' action is taking place in the workflow/config.

  • All the functions that were already marked by "# automatically passed to the function". I am adding some text at the beginning to clarify the difference btw automatically passed and manually.

Nice, I edited that a bit, too.

juliettelavoie commented 1 year ago

I'm probably too close to it and assumed people will know the options after running through the notebooks, but you are probably right. I can add commented defaults in the config for people who start directly with the template.

vindelico commented 1 year ago

I'm probably too close to it and assumed people will know the options after running through the notebooks, but you are probably right. I can add commented defaults in the config for people who start directly with the template.

I think @RondeauG mentioned that the docs / notebooks point to the templates somewhere already. I would really see the template(s) as another angle for learning and a good place to add details that the notebooks don't cover. I notice that I was hitch-hiking on your PR #233 for unrelated modifications. We should have a PR dedicated to bonification of the template(s).

juliettelavoie commented 1 year ago

Interesting idea. I really saw the templates as going after the notebooks. (Though,I just noticed that "workflow templates" is before "Good to know" and any notebooks on the RtD.) We should discuss this further at the meeting: what is the recommanded entry point ?

One thing I will say is that it much easier to showcase different arguments in the notebooks. The templates are better for showing at the mechanics of making it operational.

juliettelavoie commented 1 year ago

yes, I will create a new PR

juliettelavoie commented 1 year ago

new PR: #235