ESMValGroup / ESMValCore

ESMValCore: A community tool for pre-processing data from Earth system models in CMIP and running analysis scripts.
https://www.esmvaltool.org
Apache License 2.0
42 stars 38 forks source link

Using fx_files for preprocessor fails in rare cases #448

Closed schlunma closed 4 years ago

schlunma commented 4 years ago

Describe the bug @bettina-gier found a funny bug: In some cases, using fx files in preprocessors (e.g for landsea_fraction_weighting) fails.

This recipe works:

preprocessors:

  land_fraction_weighting: &land_fraction_weighting
    weighting_landsea_fraction: &weighting_options
      area_type: land

diagnostics:

  diag_mvi_tas:
    variables:
      tas: &var_tas
        preprocessor: land_fraction_weighting
        project: CMIP5
        mip: Amon
        exp: historical
        ensemble: r1i1p1
        start_year: 1901
        end_year: 2005
    additional_datasets: &tas_datasets
      # - {dataset: CRU, project: OBS, type: reanaly, version: TS4.02, tier: 2}
      - {dataset: bcc-csm1-1}
    scripts:
      null

  diag_two_vars_scatter:
    variables:
      cSoil:
        preprocessor: land_fraction_weighting
        project: CMIP5
        mip: Lmon
        exp: historical
        ensemble: r1i1p1
        start_year: 1986
        end_year: 2005
        additional_datasets:
          - {dataset: HWSD, project: OBS, type: reanaly, version: 1.2, tier: 3, start_year: 2000, end_year: 2000}
    scripts:
      null

This one

preprocessors:

  land_fraction_weighting: &land_fraction_weighting
    weighting_landsea_fraction: &weighting_options
      area_type: land

diagnostics:

  diag_mvi_tas:
    variables:
      tas: &var_tas
        preprocessor: land_fraction_weighting
        project: CMIP5
        mip: Amon
        exp: historical
        ensemble: r1i1p1
        start_year: 1901
        end_year: 2005
        reference_dataset: CRU
        plot_units: degC
    additional_datasets: &tas_datasets
      - {dataset: CRU, project: OBS, type: reanaly, version: TS4.02, tier: 2}
      - {dataset: bcc-csm1-1}
    scripts:
      null

  diag_two_vars_scatter:
    variables:
      cSoil:
        preprocessor: land_fraction_weighting
        project: CMIP5
        mip: Lmon
        exp: historical
        ensemble: r1i1p1
        start_year: 1986
        end_year: 2005
        reference_dataset: HWSD
        plot_units: PgC
        additional_datasets:
          - {dataset: HWSD, project: OBS, type: reanaly, version: 1.2, tier: 3, start_year: 2000, end_year: 2000}
    scripts:
      null

fails with

Traceback (most recent call last):
  File "ESMValCore/esmvalcore/_main.py", line 220, in run
    conf = main(args)
  File "ESMValCore/esmvalcore/_main.py", line 156, in main
    process_recipe(recipe_file=recipe, config_user=cfg)
  File "ESMValCore/esmvalcore/_main.py", line 202, in process_recipe
    recipe = read_recipe_file(recipe_file, config_user)
  File "ESMValCore/esmvalcore/_recipe.py", line 74, in read_recipe_file
    recipe_file=filename)
  File "ESMValCore/esmvalcore/_recipe.py", line 1032, in __init__
    self.tasks = self.initialize_tasks() if initialize_tasks else None
  File "ESMValCore/esmvalcore/_recipe.py", line 1379, in initialize_tasks
    task_name=task_name,
  File "ESMValCore/esmvalcore/_recipe.py", line 985, in _get_preprocessor_task
    name=task_name,
  File "ESMValCore/esmvalcore/_recipe.py", line 792, in _get_single_preprocessor_task
    config_user=config_user,
  File "ESMValCore/esmvalcore/_recipe.py", line 739, in _get_preprocessor_products
    config_user=config_user)
  File "ESMValCore/esmvalcore/_recipe.py", line 506, in _update_fx_settings
    fx_dict = _get_landsea_fraction_fx_dict(variable, config_user)
  File "ESMValCore/esmvalcore/_recipe.py", line 459, in _get_landsea_fraction_fx_dict
    fx_file, _ = _get_correct_fx_file(variable, fx_var, config_user)
  File "ESMValCore/esmvalcore/_recipe.py", line 424, in _get_correct_fx_file
    fx_files = _get_input_files(fx_variable, config_user)
  File "ESMValCore/esmvalcore/_recipe.py", line 551, in _get_input_files
    drs=config_user['drs'])
  File "ESMValCore/esmvalcore/_data_finder.py", line 264, in get_input_filelist
    variable['end_year'])
  File "ESMValCore/esmvalcore/_data_finder.py", line 111, in select_files
    start, end = get_start_end_year(filename)
  File "ESMValCore/esmvalcore/_data_finder.py", line 99, in get_start_end_year
    'not be read from the file'.format(filename)
ValueError: File /mnt/lustre02/work/bd0854/DATA/ESMValTool2/OBS/Tier3/HWSD/OBS_HWSD_reanaly_1.2_fx_sftlf.nc dates do not match a recognized pattern and time can not be read from the file

Note the additional OBS dataset in the second case.

I will try to address this in #439. I think this bug was introduced in #432. @valeriupredoi

valeriupredoi commented 4 years ago

that should not happen if the variable's frequency is fx as it should be, due to the condition in _data_finder.py/l.262

schlunma commented 4 years ago

That's exactly the problem. In _add_fxvar_keys the frequency gets set to mon even if the mip is fx, and the reason for this is that reading the CMOR_TABLE suddenly gives a frequency of mon instead of fx. So at some point in the code the CMOR_TABLE object get changed, but I still don't know where...

valeriupredoi commented 4 years ago

I don't see where the problem is - i used master and ran the second bit of the recipe no problemo - I think @bettina-gier is using an older copy of master or some custom branch since the code line numbers differ as well

valeriupredoi commented 4 years ago

aha! If you run the second bit only all goes well - if you run diag mvi and diag scatter I get that exact error

valeriupredoi commented 4 years ago

so if you run only the first diag group it fails with Failed to run weighting_landsea_fraction(air_temperature... coz I got no sftlf/of files - if you run only the second diagnostic group, that goes well, if you run them both you get Tina's error - it smells to me of assignigning a var frequency from one diag and keeping it for the second which is highly illegal :grin:

schlunma commented 4 years ago

Found the problem:

The problem appears when we call the the function get_variable of the CMOR table to check if the fx variable is in the table mip:

https://github.com/ESMValGroup/ESMValCore/blob/bc7c117323cd8b6eafcba72f2bcf0ebc1fc5229e/esmvalcore/_recipe.py#L404

I though that this returns always None if the variable is not in the table, e.g. for sftlf and Amon. However, for the OBS project (which is not cmor_strict), this is NOT true since the get_variable also checks the other mips:

https://github.com/ESMValGroup/ESMValCore/blob/bc7c117323cd8b6eafcba72f2bcf0ebc1fc5229e/esmvalcore/cmor/table.py#L677-L713

L. 712 causes the trouble: It overwrites the frequency of the original fx table with the the frequency of the Amon table. I guess this behavior is not intended, since l. 710 already has a var_info.copy(), but the line needs to be

var_info = var_info.copy()

to work correctly. If I do that, the bug disappears. Is that replacement correct or is this frequency change an intended behavior, @jvegasbsc ?

valeriupredoi commented 4 years ago

Great work, Inspector Poirot! I am fairly sure that's a bug - but waiting for @jvegasbsc to confirm :beer:

valeriupredoi commented 4 years ago

Prob best if you opened a PR with that fix Manu, so we see if all tests pass and give Javi a ready made platform to make changes, if needed :beer:

schlunma commented 4 years ago

All right, will do after #439 is merged

jvegreg commented 4 years ago

Is that replacement correct or is this frequency change an intended behavior, @jvegasbsc ?

You are right, @schlunma, it is not intended. In fact, the other copy call was introduced to solve a similar error.

bouweandela commented 4 years ago

All right, will do after #439 is merged

@schlunma Why is #439 needed to solve this issue? Is this a bug in the master branch or in an unmerged branch?

schlunma commented 4 years ago

This is a bug in the master branch and independent from #439. I will add a PR that fixes this.

mattiarighi commented 4 years ago

@schlunma I tried your second recipe above (the one with OBS) but it fails:


multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/mnt/lustre02/work/bd0080/b309057/SOFTWARE/miniconda3/envs/esmvaltool/lib/python3.8/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 723, in _run_task
    output_files = task.run()
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 242, in run
    self.output_files = self._run(input_files)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 428, in _run
    product.apply(step, self.debug)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 296, in apply
    self.cubes = preprocess(self.cubes, step, **self.settings[step])
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 241, in preprocess
    result.append(_run_preproc_function(function, item, settings))
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 224, in _run_preproc_function
    return function(items, **kwargs)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/_weighting.py", line 86, in weighting_landsea_fraction
    raise ValueError(
ValueError: Weighting of 'tas' with 'land' fraction failed because of the following errors: File for 'sftlf' not found. File for 'sftof' not found.
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_main.py", line 242, in run
    conf = main(args)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_main.py", line 176, in main
    process_recipe(recipe_file=recipe, config_user=cfg)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_main.py", line 228, in process_recipe
    recipe.run()
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_recipe.py", line 1346, in run
    run_tasks(self.tasks,
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 643, in run_tasks
    _run_tasks_parallel(tasks, max_parallel_tasks)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 688, in _run_tasks_parallel
    _copy_results(task, running[task])
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 711, in _copy_results
    task.output_files, updated_products = future.get()
  File "/mnt/lustre02/work/bd0080/b309057/SOFTWARE/miniconda3/envs/esmvaltool/lib/python3.8/multiprocessing/pool.py", line 771, in get
    raise self._value
  File "/mnt/lustre02/work/bd0080/b309057/SOFTWARE/miniconda3/envs/esmvaltool/lib/python3.8/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 723, in _run_task
    output_files = task.run()
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 242, in run
    self.output_files = self._run(input_files)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 428, in _run
    product.apply(step, self.debug)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 296, in apply
    self.cubes = preprocess(self.cubes, step, **self.settings[step])
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 241, in preprocess
    result.append(_run_preproc_function(function, item, settings))
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/__init__.py", line 224, in _run_preproc_function
    return function(items, **kwargs)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/preprocessor/_weighting.py", line 86, in weighting_landsea_fraction
    raise ValueError(
ValueError: Weighting of 'tas' with 'land' fraction failed because of the following errors: File for 'sftlf' not found. File for 'sftof' not found.
2020-06-22 09:35:56,442 UTC [44474] INFO    If you suspect this is a bug or need help, please open an issue on https://github.com/ESMValGroup/ESMValTool/issues and attach the run/recipe_*.yml and run/main_log_debug.txt files from the output directory.
schlunma commented 4 years ago

Sorry, I forgot to mention that you have to add the option exclude: ['CRU'] to the weighting_landsea_fraction preprocessor.