-
Notifications
You must be signed in to change notification settings - Fork 45
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
Added modular functions to model the production of Green Steel: #190
base: dev/refactor
Are you sure you want to change the base?
Conversation
…nd HDRI are modeled, tests for every function accompanied as well
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.
This looks pretty good. I would like to see an example or more formal documentation for this so it can be used more easily later. This could be a simple readme in hopp/simulation/technologies/steel
(minimum) or, what I would prefer, would be a python script or Jupyter notebook walking through how to use the code.
We also need to figure out how we can best hook this up with HOPP directly.
carbon_total = outputs[2] | ||
lime_total = outputs[3] | ||
|
||
assert pytest.approx(steel_output_actual) == 1044.68 |
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.
These should ideally be broken out so all tests run.
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.
Broken
direct_emissions = outputs[2] | ||
total_emissions = outputs[3] | ||
|
||
assert pytest.approx(indirect_emissions_total, 0.1) == 325.56 |
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.
Should be broken up so all tests run
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.
Broken
lime_cost_total = outputs[7] | ||
total_emission_cost = outputs[8] | ||
|
||
assert pytest.approx(eaf_total_capital_cost) == .42 |
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.
Should be broken up
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.
Broken up
mass_h2_h2o_output = outputs[9] | ||
mass_iron_ore_output = outputs[10] | ||
|
||
assert pytest.approx(steel_out_desired) == steel_output_desired |
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.
Should be broken up
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.
broken up
labor_cost = outputs[6] | ||
|
||
assert pytest.approx(capital_cost) == .24 | ||
assert pytest.approx(operational_cost) == .013 |
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.
should be broken up
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.
broken up
@@ -0,0 +1,73 @@ | |||
import pytest | |||
from hopp.simulation.technologies.steel.eaf_model import eaf_model |
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 add comments to each test regarding where the values came from or how they were determined
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.
Added in doc strings in test files
H_t: Enthalpy of element at given temp (kj/g) | ||
''' | ||
|
||
def h2_enthalpy_1(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.
I don't see any tests for these functions.
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.
Added tests for these functions. These functions could be added to but will take time to implement all
H=0 | ||
H_t=A*t +(B*t*t)/2 +(C*t*t*t)/3 + (D*t*t*t*t)/4-(E/t)+F-H | ||
return H_t | ||
def h2_enthalpy_2(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.
The h2_enthalpy_x functions should be a single function with an if statement to determine which method to use. You could to this by writing a third function called h2_enthalpy
that then calls the existing two functions, or you can just combine the contents of the two functions.
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.
Merged into one function with if statements
H_t=(A*t +(B*t*t)/2 +(C*t*t*t)/3 + (D*t*t*t*t)/4-(E/t)+F-H)/mol_weight_H2O | ||
return H_t | ||
|
||
def fe_enthalpy_1(T):#298-1809 K |
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.
same comment on using a single function as I gave in the h2_enthalpy_x functions
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.
Merged into one function with if statements
H=-74.87310 | ||
H_t=(A*t +(B*t**2)/2 +(C*t**3)/3 + (D*t**4)/4-(E/t)+F-H)/mol_weight_CH4 | ||
return H_t | ||
def ch4_enthalpy_2(T):# T in range 1300-6000 K |
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.
Should also combine the ch4 functions. You may want to throw a value error in case a temperature given is out of range (same for other temperature input functions here). That is a lot safer than just a comment.
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.
Combined ch4 functions as well
Will add main on the bottom of hdri and eaf and will create a h2_main run script to show how I implemented the code |
… function. Updated error downstream
H=7.788015 | ||
H_t=(A*t +(B*t*t)/2 +(C*t*t*t)/3 + (D*t*t*t*t)/4-(E/t)+F-H)/mol_weight_fe | ||
|
||
if T >= 1809: #fe_2 1809 - 3133K |
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.
should be elif
H=0 | ||
H_t=(A*t +(B*t*t)/2 +(C*t*t*t)/3 + (D*t*t*t*t)/4-(E/t)+F-H)/mol_weight_H2 | ||
|
||
if T >= 1000: #h2_2 #T in range 1000-2500 K |
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.
should be elif
### Water enthalpy coefficients | ||
|
||
def h2o_enthalpy(T): | ||
mol_weight_H2O=18.0153 | ||
def h2o_enthalpy(T): # 500-1700 |
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.
needs temperature value error check
def feo_enthalpy(T): | ||
mol_weight_feo=71.844 #in grams | ||
mol_weight_feo=71.844 #in grams 298-1650 |
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.
it looks like this and other value ranges should have been error checks on temperature, not in the comment on molecular weight
H=-74.87310 | ||
H_t=(A*t +(B*t**2)/2 +(C*t**3)/3 + (D*t**4)/4-(E/t)+F-H)/mol_weight_CH4 | ||
|
||
if T >1300: #2 1300-6000 K |
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.
should be elif
if T < 298 or T > 6000: | ||
raise ValueError(f"Inputted temperatute {T} for methane gas is out of range of 298-6000 K") | ||
|
||
if T <= 1000: #1 298-1300 K |
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.
should be elif
if T < 298 or T > 2500: | ||
raise ValueError(f"Inputted temperatute {T} for hydrogen gas is out of range of 298-2500 K") | ||
|
||
if T < 1000: #h2_1 # T1 and T2 should be in the range of 298-1000 K <br> |
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.
elif
…ied by example file in eaf and hdri models. Fixed test files and hand verified results
…e. Added tests for error messages
EAF and HDRI are modeled, tests for every function accompanied as well. Created from scratch so no history on these files. Will add name == 'main': on these after I understand how these will be used in larger picture