OMS-NetZero / FAIR

Finite-amplitude Impulse Response simple climate model

https://docs.fairmodel.net

Apache License 2.0

121 stars 61 forks source link

Fix scmdf_to_emissions #77

Closed znicholls closed 4 years ago

znicholls commented 4 years ago

Pull request

Please confirm that this pull request has done the following:

[x] Tests added
[x] Documentation added (where applicable)
[x] Example added (either to an existing notebook or as a new notebook, where applicable) (N/A)
[x] Description in CHANGELOG.rst added

Adding to CHANGELOG.rst

Please add a single line in the changelog notes similar to one of the following:

- (`#XX <http://link-to-pr.com>`_) Added feature which does something
- (`#XX <http://link-to-pr.com>`_) Fixed bug identified in (`#XX <http://link-to-issue.com>`_)

znicholls commented 4 years ago

You might see a way to speed things up, but this works.

znicholls commented 4 years ago

@chrisroadmap thinking about it more, would it be simpler if we moved the scmdf_to_emissions function into climate-assessment?

chrisroadmap commented 4 years ago

@chrisroadmap thinking about it more, would it be simpler if we moved the scmdf_to_emissions function into climate-assessment?

nah, leave it here, another interface possibility and we'll be ready for scmdata world domination

znicholls commented 4 years ago

beauno hopefully this scmdata makes more sense now you've seen it in action...

znicholls commented 4 years ago

just ye, check the units...

chrisroadmap commented 4 years ago

You might see a way to speed things up, but this works.

This takes 4 seconds per scenario for me :(

znicholls commented 4 years ago

This takes 4 seconds per scenario for me :(

Yep. There's ways to speed it up but I felt that working out what is going on first (and making sure it gives the right numbers) was priority, with optimisation coming thereafter.

znicholls commented 4 years ago

If you get it working how you want, I'll speed it up tomorrow

chrisroadmap commented 4 years ago

The aerosol problem was nonsensical defaults in openscm_runner, so this branch is numerically fine I think. Just needs a 40x speedup...

znicholls commented 4 years ago

The aerosol problem was nonsensical defaults in openscm_runner, so this branch is numerically fine I think. Just needs a 40x speedup...

Beauty. Keep in mind that this only has to happen once per scenario in openscm_runner so if it takes 1 second (which yes is 4x speedup but I don't think that'll be crazy difficult), it's not so much compared to the 600 model runs?

chrisroadmap commented 4 years ago

The aerosol problem was nonsensical defaults in openscm_runner, so this branch is numerically fine I think. Just needs a 40x speedup...

Beauty. Keep in mind that this only has to happen once per scenario in openscm_runner so if it takes 1 second (which yes is 4x speedup but I don't think that'll be crazy difficult), it's not so much compared to the 600 model runs?

Yeah, true. It does make testing a bit of a pain in the ass. If you can make it faster then great but put it as low priority.