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 issue #20: when sanity_check_hierarchy is called with only Emissions|CO2 input data #23

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jkikstra
Copy link
Collaborator

@jkikstra jkikstra commented Dec 13, 2022

Note: follows from #19 and accompanying PR #22 - so let's first merge that one (if not based on that, it is not possible to reproduce the original error that we address here).

For full description of error, see #20.

  • Tests added
  • Documentation added/changed if necessary
  • Example added (in the documentation, to an existing notebook, or in a new notebook)
  • Description in CHANGELOG.rst added (single line such as: (`#XX <https://github.com/iiasa/climate-assessment/pull/XX>`_) Added feature which does something)

@jkikstra
Copy link
Collaborator Author

Wrote an ad-hoc fix, but not so sure if this works in the desired way for all cases.

Using input file , we get the input co2_infill_db as object passed to sanity_check_hierarchy(): ar6_minimum_emissions_co2_infill_db.csv
This has "Emissions|CO2" for both scenarios, even though the input GCAM 5.3 - NGFS2_Current Policies scenario only has "Emissions|CO2|Energy and Industrial Processes"

Maybe @znicholls would like to have a look at the desired behaviour here?

@znicholls
Copy link
Collaborator

Maybe @znicholls would like to have a look at the desired behaviour here?

I've had a quick look. I'd say let's sort out #19 first, then we can add the test you have added here and work out how to pass it. It is a confusing maze of stuff in all these checks (e.g. I have no idea why we return total CO2 from the infilling database in run_infilling) so I think we'll have to just do something fairly ad-hoc to workaround it, then we deal with it properly as part of a larger refactoring (raises again the question of whether to invest more serious time into such an effort).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants