-
Notifications
You must be signed in to change notification settings - Fork 41
Improve aluminum model #345
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
Conversation
Thanks @macflo8! The error message on CI is a little opaque, but I believe it occurs because one of the added/modified files wants to import |
Yes, good catch. I had a conflict during rebasing and did not merge correctly it seems. I will try to repeat the step and clean that up. |
90f82ff
to
9f744c6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
=======================================
- Coverage 78.6% 73.4% -5.3%
=======================================
Files 219 220 +1
Lines 17499 17746 +247
=======================================
- Hits 13766 13034 -732
- Misses 3733 4712 +979
🚀 New features to boost your workflow:
|
07c1458
to
a62e0e0
Compare
a62e0e0
to
ae03a7c
Compare
Remove rows where year_vtg - year_act is greater than the longest lifetime
- Move SSP differentiated parameters from input data to functions - Remove duplicated input files for SSPs - Change input data format from xlsx to csv - Move unformatted data files into subdirectory
- Move SSP differentiated parameters from input data to functions - Remove duplicated input files for SSPs - Change input data format from xlsx to csv - Move unformatted data files into subdirectory
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 @macflo8 for the work and collaboration here!
All checks are passing (except one, see below), so approved.
Just to recap some points we discussed for follow-up:
- We accept the drop in coverage for the time being—to be addressed in a follow-up PR after other material pieces are on
main
. - Fetch aluminum data from upstream sources #348
- We can investigate how to replace .material.util.read_config() with a Config class, to avoid the Context.get_instance(-1) call that seems fragile on GitHub Actions/in the test suite. This should in turn allow to remove the one XFAIL test mark added in this PR.
I will now:
- Rebase the PR on latest
main
. - Merge (without waiting for the checks to all run again).
- Rebase the
ssp-dev
branch of Collect pieces for SSP 2024/ScenarioMIP #235 onmain
.
152507c
to
c4ce855
Compare
This PR adds the improvements of the aluminum sector representation in MESSAGEix-Materials developed in the course of the ScenarioMIP project. It cherry-picks some initial commits from the #235 branch.
How to review
developer (someone like the reviewer) will be able to understand what the code
does in the future.
PR checklist