-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add chemicals industry (new) #418
base: develop
Are you sure you want to change the base?
Add chemicals industry (new) #418
Conversation
My high-level questions, just to confirm:
|
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.
Nice job! However, I have several suggestions.
Biggest one is that I feel like you are not using the correct JRC function for the electricity consumption stuff. See the specific comment. Feel free to comment if you think I got something wrong!
message: "Calculate energy demand for the 'Chemicals Industry' sector in JRC-IDEES." | ||
conda: CONDA_PATH | ||
# params: | ||
# config = config["params"]["config-chemicals-industry"] |
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.
Consider looking at which parameters should be user configurable and bring them to this level.
""" | ||
# Get the electricity consumption (assuming every demand that could be met by electricity is met by electricity) | ||
electrical_consumption = ( | ||
(jrc.replace_carrier_final_demand("Electricity", jrc_energy)) |
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.
I feel in my bones that this is not the function you want to call here.
Check get_section_final_intensity_auxiliary_electric
, which is what the steel sector uses.
This function gets electricity demand for stuff like motor drives, pumps and lighting.
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.
The best way to answer this is probably looking at what the old code does for both chemicals and steel here, not "other" industry, which is unique.
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.
I feel in my bones that this is not the function you want to call here. Check
get_section_final_intensity_auxiliary_electric
, which is what the steel sector uses.This function gets electricity demand for stuff like motor drives, pumps and lighting.
As I understand, the function you suggest is giving an intensity value, i.e. twh/t. But what's needed here, at least according to the old code, is the absolute value of energy in twh (just an example of unit).
chem_co2_consumption = chem_co2_consumption.assign_attrs(units="t").assign_coords( | ||
carrier_name="co2" | ||
) | ||
# TODO: find out how co2 is called in jrc |
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.
As far as I remember, CO2 is not present in JRC, same with "methanol" above.
Instead, follow the same naming convention JRC uses (capital letters, etc).
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.
Yeah, this is why we provide CO2 demand as constants in a dictionary earlier in the script.
) | ||
chem_energy_consumption = chem_energy_consumption.assign_attrs( | ||
units="twh" | ||
).assign_coords(carrier_name="electricity") |
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.
Please keep the same JRC naming convention. See the iron and steel sector.
.sum(dim=["section", "carrier_name"]) | ||
.drop_vars(["subsection", "cat_name"]) | ||
.assign_attrs(units="twh") | ||
.assign_coords(carrier_name="space_heat") |
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.
More naming conventions. Please keep "Low enthalpy heat".
) | ||
|
||
# Prettify | ||
total_consumption = jrc.standardize(total_consumption, "twh, t", "demand") |
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.
Since Chemicals industry is unique in that it also outputs demand in tonnes, please create a separate xarray variable for it with a clear and short name (see the jrc_energy
dataset as an example).
This is to avoid ambiguous units: right now you cannot know were twh or tonnes apply unless you ask the person who made the code!
useful_dem_tot = jrc_energy["useful"].sum("carrier_name") | ||
useful_dem_tot = jrc_energy["useful"].sum( | ||
"carrier_name" | ||
) # MC note: including all carriers |
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.
Please remove these comments.
If the function is not descriptive enough, improve the docstring.
Easier to maintain down the line (and avoids black
creating extra lines because of the comment).
assert useful_dem_tot.sum() < carrier_final_demand.sum(), "Creating energy!" | ||
|
||
# assert useful_dem_tot.sum() < carrier_final_demand.sum(), "Creating energy!" # MC note: but useful_dem_tot includes all carrier demands while carrier_final_demand only contains the demand related to one specific carrier. Surely this is fair comparison? | ||
# MC note: commented out because this is not correct for chemicals industry. but need to coordinate with Ivan regarding how this function should be written. |
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.
See my comment in the "other industry" PR:
#355 (comment)
I'll try to help here, but I am unfamiliar with the chemicals section of the code. I can only help with the jrc helper functions and so on. If there are more questions, I can help when I'm back from my vacation.
|
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 this @tud-mchen6! I would restructure the chemicals industry script a bit to move constants around and to distribute it into smaller functions that you can then give nice docstrings for (for future maintainability, e.g. so others don't wonder the same things you have wondered when making this contribution!)
from utils import filling | ||
from utils import jrc_idees_parser as jrc | ||
|
||
H2_LHV_KTOE = 2.863 # 0.0333 TWh/kt LHV -> 2.863ktoe/kt |
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.
Some of these values are also in the steel script. To avoid the maintenance burden, perhaps we move these to a single file (e.g. utils/constants.py
?)
will be required for iron ore processed using H-DRI, but is not required by EAF. | ||
|
||
This function does the following: | ||
1. |
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.
Fill?
"Methanol": 1.373, | ||
} | ||
|
||
energy_demand = { # MWh/t |
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.
Move to config (there are user assumptions here)
) | ||
|
||
# Gather relevant params for Basic chemicals in the Chemicals Industry | ||
h2_demand = { # t/t |
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.
move to top of file (they are constants)
"Ammonia": 0.178, | ||
"Methanol": 0.189, | ||
} | ||
methanol_demand = { |
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.
move to top of file (they are constants)
"Ammonia": 17, | ||
"Methanol": 2, | ||
} | ||
molar_mass = { |
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.
move to top of file (they are constants)
"Methanol": 1.5, # auxiliary demand, could just be assumed as already included in existing electricity demand | ||
} | ||
|
||
mass = { # Bazzanella and Ausfelder, 2017 |
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.
move to top of file (they are constants)
"Ammonia": 17.01, | ||
"Methanol": 32.04, | ||
} | ||
moles = {k: mass[k] / molar_mass[k] for k in mass} |
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.
I would separate the mass-to-molar mass calc into its own function
new_chemicals_demand.to_netcdf(output_path) | ||
|
||
|
||
def transform_jrc_chemicals_subsector_demand( |
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.
I don't think we're using "subsector" any more. See the steel script
chem_co2_consumption = chem_co2_consumption.assign_attrs(units="t").assign_coords( | ||
carrier_name="co2" | ||
) | ||
# TODO: find out how co2 is called in jrc |
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.
Yeah, this is why we provide CO2 demand as constants in a dictionary earlier in the script.
In answer to your questions:
We completely overhaul the chemical industry stuff and as a result only keep direct electricity consumption, which refers to all energy requirements to process chemicals. I believe that the non-electricity consumption in JRC-IDEES refers to creating relevant chemicals, which we completely replace with our own demands for CO2, hydrogen, and auxiliary electricity. This probably requires some review though.
It gets turned into "space heat demand" later, which can be met by any available technologies for that.
Electricity.
The electricity demand from jrc_energy refers to demand to process basic chemicals. The electricity demand we get from processing jrc_prod is for creating those basic chemicals. They should be mutually exclusive sets of demands that can be added together without double counting. |
Fixes #311. This is the newer version; discarding #348. Everything works, but still in progress of prettifying.
Checklist
Any checks which are not relevant to the PR can be pre-checked by the PR creator. All others should be checked by the reviewer. You can add extra checklist items here if required by the PR.