search

iiasa / climate-assessment

https://climate-assessment.readthedocs.io/en/latest
MIT License
19 stars 18 forks source link

Specify percentiles in climate assessment #24

Open znicholls opened 1 year ago

znicholls commented 1 year ago

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.

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 commented 1 year ago

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

orichters commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

znicholls commented 6 months ago

It was so long ago, I have no idea. If you know the behaviour you want, chuck in a test, get it passing then should be straight forward from there.

On Tue, Feb 20, 2024, 10:16 orichters @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/iiasa/climate-assessment/pull/24#issuecomment-1953782076, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFUH5GYJOOOE2LHDTNUAP23YURSXJAVCNFSM6AAAAAATAKIWJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJTG44DEMBXGY . You are receiving this because you were mentioned.Message ID: @.***>