Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix small errors in the arctic_ocean diagnostic #1722

Merged
merged 3 commits into from Jul 20, 2020
Merged

Conversation

koldunovn
Copy link
Contributor

@koldunovn koldunovn commented Jul 13, 2020

Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • Give this pull request a descriptive title that can be used as a one line summary in a changelog
  • Make sure your code is composed of functions of no more than 50 lines and uses meaningful names for variables
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Preferably Codacy code quality checks pass, however a few remaining hard to solve Codacy issues are still acceptable. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • Please use yamllint to check that your YAML files do not contain mistakes

Closes #1704

@koldunovn
Copy link
Contributor Author

koldunovn commented Jul 13, 2020

Tests are failing in the same way as for #1721 Looks like something related to pytest.

@mattiarighi
Copy link
Contributor

mattiarighi commented Jul 13, 2020

The recipe is still broken, but now with a different error:

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/_task.py", line 520, in _run
    self._collect_provenance()
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_task.py", line 583, in _collect_provenance
    if ancestor_file in ancestor_products:
TypeError: unhashable type: 'list'

This is the same as reported in #1711.

@koldunovn
Copy link
Contributor Author

koldunovn commented Jul 13, 2020

@mattiarighi Works with stable esmvalcore :) Will try to diagnose what's happening with the master of esmvalcore tomorrow.

@mattiarighi
Copy link
Contributor

mattiarighi commented Jul 13, 2020

Thank you, that's good to know.

@koldunovn
Copy link
Contributor Author

koldunovn commented Jul 15, 2020

@mattiarighi The latest commit make the diagnostic run witht he lates esmvalcore master. However provenance should properly process None ancestor files as described in ESMValGroup/ESMValCore#711 (comment)

There are a lot of warnings about the wrong files in the provenance. I don't know the mechanics of the provenance, but looks like that it thinks that valid fileas are only those created by the preprocessor. I use a lot of intermediate files, which are not recognised, so that's were the warnings are comming from.

@mattiarighi
Copy link
Contributor

mattiarighi commented Jul 15, 2020

I think we need to wait for @bouweandela to sort this out. Thanks for the feedback!

@bouweandela
Copy link
Member

bouweandela commented Jul 20, 2020

The latest commit make the diagnostic run witht he lates esmvalcore master. However provenance should properly process None ancestor files as described in ESMValGroup/ESMValCore#711 (comment)

To better understand this issue, can you describe what kind of files this are? I expected all output files to be somehow derived from the input data, but maybe I missed something?

There are a lot of warnings about the wrong files in the provenance. I don't know the mechanics of the provenance, but looks like that it thinks that valid fileas are only those created by the preprocessor. I use a lot of intermediate files, which are not recognised, so that's were the warnings are comming from.

The design decision we made when implementing the provenance was that we would not add this level of detail. Valid files are those that are created by ancestor tasks, in most cases this is/are indeed the preprocessor tasks (see here for an introduction). However, this was not communicated clearly enough in the documentation. That's why I recently added the warnings, to better guide diagnostic developers in choosing the correct files.

@koldunovn
Copy link
Contributor Author

koldunovn commented Jul 20, 2020

To better understand this issue, can you describe what kind of files this are? I expected all output files to be somehow derived from the input data, but maybe I missed something?

In my case it's a map of the transect. It is generated from the transect positions, that are defined in the code (

)

The design decision we made when implementing the provenance was that we would not add this level of detail. Valid files are those that are created by ancestor tasks, in most cases this is/are indeed the preprocessor tasks (see here for an introduction). However, this was not communicated clearly enough in the documentation. That's why I recently added the warnings, to better guide diagnostic developers in choosing the correct files.

Thanks for clarigying it. Now as the diagnostic works do we need more actions?

@mattiarighi mattiarighi merged commit bcfed57 into master Jul 20, 2020
1 check passed
@mattiarighi mattiarighi deleted the arctic_ocean_fix branch Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

recipe_arctic_ocean.yml fails to run
3 participants