-
Notifications
You must be signed in to change notification settings - Fork 15
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
v0.41.0 master < develop #357
Conversation
using Cambium instead of AVERT for CO2 emissions by default
Based on Cambium 2022 Mid-Case Scenario, LRMER CO2e (Combustion+Precombustion) 2024-2049 values
no longer an output under Site
under ElectricUtility
end use or busbar
This will prevent a Cambium API call with each unit test
For tests: - changed "region_abbreviation" to "avert_region_abbreviation" - Updated RE and Emissions expected values to account for alignment between load year and emissions year (tested with assumed AVERT emissions year of 2017 to ensure that this is why the emissions values changed) - Added 0 grid emissions to scenario.jl when off_grid_flag is true
Fix rate adjustments
Cambium Grid Climate Emissions and Updated AVERT
@adfarth have you run test_with_xpress.jl locally? |
Thank you for the reminder! I had not, but now I have and all tests have passed locally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there breaking changes other than expected differences in emissions accounting and results when doing emissions reduction goals or CO2 cost in the optimization?
Otherwise, just a couple of questions about runtests.jl versus test_with_xpress.jl.
test/runtests.jl
Outdated
@@ -1895,6 +1982,7 @@ else # run HiGHS tests | |||
end | |||
end | |||
|
|||
# TODO: refresh tests and test values as needed after Cambium update; revised values are in test_with_xpress.jl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_with_xpress.jl is using a different/updated Cambium version/data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Bill-Becker These are the emissions tests that Alex commented out a while back because they weren't compatible with the HiGHS solver. The test values are slightly updated because of the AVERT data update that we made in this PR. Alex added this note to indicate that once we are able to run these tests with a non-Xpress solver, we should update the values here. Does that all make sense?
test/test_with_xpress.jl
Outdated
@@ -1377,6 +1377,82 @@ end | |||
@test calculated_ghp_capex ≈ reopt_ghp_capex atol=300 | |||
end | |||
|
|||
@testset "Cambium Emissions" begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the same test as in "runtests.jl" with the HiGHS solver? If so, we don't need to add it here. We should be prioritizing adding tests to runtests.jl with an open source solver whenever possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this was exactly the same as the one in runtests. I deleted this testset in test_with_xpress
in cb6d9d9
@adfarth also, just confirming again that you ran test_with_xpress.jl locally with this develop branch. |
Yes, confirmed that I ran this on the develop branch. |
Summary of changes at a high level:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the responses - they all look good. Related to the discussion of Xpress versus HiGHS tests - does anything you added with this PR require Xpress to work properly? We are about to take Xpress down from the publicly available API.
Not that I can think of. I think there were some emissions modeling capabilities that were timing out or failing with HiGHS (hence the commented out tests in runtests), but those are not explicitly related to any change introduced by this PR (this is just changing the data sources, not any constraints) |
See #352 and #235