Skip to content

Specify percentiles in climate assessment#24

Open
znicholls wants to merge 5 commits into
mainfrom
percentiles-climate-assessment
Open

Specify percentiles in climate assessment#24
znicholls wants to merge 5 commits into
mainfrom
percentiles-climate-assessment

Conversation

@znicholls

@znicholls znicholls commented Dec 16, 2022

Copy link
Copy Markdown
Collaborator

Exposes the ability to specify the percentiles, peak percentiles and temperature thresholds to calculate to the user. This is one of many places where we will need to update our interfaces. It's currently a mess, but this is fairly isolated and does the job. Fixing up the entire architecture can be a job for another day.

  • Tests added
  • Documentation added
  • 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)

Some further context

I would have written a unit test for this, but it's way too painful given how long the climate_assessment function is. That is unfortunate as this is a pretty expensive way to test such a small change.

In general, my theory on testing is that, as far as possible, you want to use unit tests. The reason is that unit tests are faster and cheaper in almost all cases. That's particularly obvious for us, where running the climate models is relatively expensive for many of the tests we're doing. This is a good concrete example, all we really want to test is that the percentiles are passed up and down correctly, we don't care about the rest and we really don't want to be writing stuff to disk (writing to disk is always slow and painful).

@znicholls znicholls changed the title Percentiles climate assessment Specify percentiles in climate assessment Dec 16, 2022
@znicholls znicholls requested a review from phackstock December 16, 2022 00:47
@znicholls

Copy link
Copy Markdown
Collaborator Author

@geiges fyi if you want to take a look/try this version out in your own runs and see if it behaves

@znicholls znicholls marked this pull request as ready for review December 16, 2022 01:10
@orichters

Copy link
Copy Markdown

What is the status of this PR? It would be very helpful if the percentiles calculated in the output could be adjusted by the user. We would like to have that for our NGFS output that goes to Climate Analytics. I could contribute to make that happen, if necessary (and I have the compentences…). Thanks!

@znicholls

Copy link
Copy Markdown
Collaborator Author

Feel free to branch off, rebase onto master (or merge master into this branch, up to you) and make an MR. The project is having a few issues with getting all the tests passing (nightly runs still failing) and having enough people to keep it all moving so if you have NGFS funding to spend time on it then please feel free. I'm happy to review any MR.

@orichters

Copy link
Copy Markdown

Thanks, @znicholls: To me, your PR looks as if that would basically work (only TODO is documentation). Is that a correct assessment?

@znicholls

znicholls commented Feb 22, 2024 via email

Copy link
Copy Markdown
Collaborator Author

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