rffscghg / MimiGIVE.jl

MIT License
9 stars 9 forks source link

CO2 SCGHG sector SCC is 0 in MimiGIVE 2.0 #67

Open hforew opened 2 weeks ago

hforew commented 2 weeks ago

When running the scghg replication procedure, the sector SCC is 0 in MimiGIVE version = "2.1.1-DEV". For version = "1.0.1-DEV", the output is as expected---sector SCC is not 0.

The source of issue may be the variable v.ce_sccs as output in results[:scc], when running estimate_give_scghg.jl.

For version = "1.0.1-DEV", v.ce_sccs is non-zero for sectors. For version = "2.1.1-DEV", v.ce_sccs is zero for sectors.

See detail below.

version = "1.0.1-DEV"

sector discount_rate scghg cromar_mortality 2.0% Ramsey 130 energy 2.0% Ramsey 8 slr 2.0% Ramsey 2 total 2.0% Ramsey 243 agriculture 2.0% Ramsey 103

sector discount_rate scghg cromar_mortality 2.0% Ramsey 0 agriculture 2.0% Ramsey 0 energy 2.0% Ramsey 0 total 2.0% Ramsey 243 slr 2.0% Ramsey 0

Note, the total is identical across versions, 243, whereas the sector SCC is not.

And the difference between two outputs appears to be v.ce_sccs as output in results[:scc]. See two images:

version = "1.0.1-DEV" ver_1 0 1-DEV_v ce_sccs

version = "2.1.1-DEV" (note, this image does not correspond to csv SCC output) ver_2 1 1-DEV_v ce_sccs0

lrennels commented 1 week ago

Hi @hforew thank you for the heads-up, this is helpful. You're right it looks like there's a bug in this new version that is incorrectly returning zeros for region = :globe and sector = (not total). I will put this on my high priority list for this week.

In general, I will note that if one is trying to perfectly replicate that repository, it's important to use the environment activation instructions, which will give you MimiGIVE v1.0.0 in the environment. By switching major versions ie. going from v1 to v2 we do not promise backwards compatibility, but authors of projects like the one you reference are safe because the Manifest.toml specifies which version of MimiGIVE they used in their analysis.

I know you're doing newer work so that probably doesn't apply to you, as you're likely using that code as a skeleton to move forward, but worth noting.

I'd advise sticking to pinned MimiGIVE v1.1.0 or v1.0.0 (just pre-2.0) while we fix this.

lrennels commented 1 week ago

@bryanparthum just a heads up that this is a bug -- only impacts v2.0.0 and onwards -- will try to fix this this week.

lrennels commented 1 week ago

@bryanparthum update it was the same thing as we had with biodiv, accessing values that were assigned undef and then not filled in -- sneaky -- will fix here #68 more elegantly and then merge

lrennels commented 1 week ago

@hforew if you'd like to test the solution, I think that #68 should have fixed it now.

I want to add some model testing so we don't run into this again, and then will have my co-developers double check things so it may take a few days to a week to have this merged. In the meantime if you are comfortable you can use that branch and feel free to let me know if anything looks off to you.

Thanks for the open-source dev help!

hforew commented 1 week ago

@lrennels Great, thank you for the quick update and explanation about specifying the version in Manifest.toml.

I will comment here once I've been able to rerun estimate_give_scghg.jl with v2.0.0.

lrennels commented 1 week ago

Sounds good! Yes, when trying to replicate a project it's best to use their environments (and the activate instructions in the README) to make sure you get the same package versions etc.

Again I know in this case you're working on something new, so you might be using more updated versions, new packages, etc. not just replicating, so makes sense that you would want to use MimiGIVE2.1.0 and other newer versions.

hforew commented 1 week ago

Thanks @lrennels. As update, after running running "add MimiGIVE#ce-fix" in package mode, I'm able to now see non-zero sectoral SCC values.

lrennels commented 4 days ago

Great to hear @hforew! I'll get this merged into master sometime next week after my co-developers take a second look.