NRRI Mines - #840
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
elenya-grant
left a comment
There was a problem hiding this comment.
Pretty high-level review and mostly small stuff! I think this looks good! Suggestions were mainly for clarity and perhaps some simplicity - nothing crazy though! Will review again if you want!
| @@ -0,0 +1,13 @@ | |||
| units,process,Tilden,Hibbing,United,Minorca,Northshore | |||
| WLT/Yr,ROM Ore,60360000,60466666.67,38012500,16566666.67,28960000 | |||
| WLT/Yr,Crushed Ore,20500000,28083000,14920000,8460000,16860000 | |||
There was a problem hiding this comment.
would it make sense to have these units expressed in OpenMDAO units? Or to have another units column that has the openmdao units? (long ton = 2240 lbs, just a note)
There was a problem hiding this comment.
should this go in a doc page?
| prob.set_val("comp.electricity_in", [annual_electricity] * 8760, units="kW") | ||
| prob.set_val("comp.fuel_in", [annual_fuel] * 8760, units="MMBtu/h") |
There was a problem hiding this comment.
should these be divided by 8760? Like [annual_fuel/8760]*8760
There was a problem hiding this comment.
I changed the name, it was just a holdover from the other iron test
| int(plant_config["plant"]["plant_life"]) | ||
| int(plant_config["plant"]["simulation"]["n_timesteps"]) |
| # check pellet production | ||
| with subtests.test("iron_ore_out"): | ||
| iron_ore_out = prob.get_val("comp.iron_ore_out", units=commodity_rate_units) | ||
| assert np.sum(iron_ore_out) == pytest.approx(7457805 * 0.98 * 1.016, rel=1e-3) |
There was a problem hiding this comment.
could ya comment somewhere what 0.98 and 1.016 are?
| """ | ||
|
|
||
| max_ore_production_rate_tonnes_per_hr: float = field() | ||
| mine: str = field(validator=contains(["Hibbing", "Northshore", "United", "Minorca", "Tilden"])) |
There was a problem hiding this comment.
would it be easier to do lower-case and then have a converter converter=(str.lower, str.strip)
mine: str = field(converter=(str.lower, str.strip), validator=contains(["hibbing", "northshore", "united", "minorca", "tilden"]))There was a problem hiding this comment.
If you'd prefer it that way or have a justification for why this is better, I could change it? I don't really have strong feelings on the matter, it's just how it was done in the other mine model
| if system_capacity * 8760 > ref_pellets: | ||
| msg = ( | ||
| f"System capacity of {system_capacity} t/h exceeds the reference pellet" | ||
| f" production of {ref_pellets} t/h." | ||
| f" This may lead to unrealistic results." | ||
| ) | ||
| warnings.warn(msg, UserWarning) |
There was a problem hiding this comment.
would it not make sense to just scale the consumptions based on the ratio of system capacity and ref_pellets? Like scale_fac = (system_capacity/(8760)) / ref_pellets)
There was a problem hiding this comment.
It does scale based on the system capacity, this is just a user warning if someone's trying to exceed the mines capacity for some reason, since I was assuming that wouldn't really be possible
| processed_ore_production = np.minimum.reduce( | ||
| [ | ||
| processed_ore_from_fuel, | ||
| processed_ore_from_electricity, | ||
| ] | ||
| ) |
There was a problem hiding this comment.
this kind of logic normally exists within technologies with a control classifier of dispatchable - but I think that it makes sense here even though this is a flexible tech. Just noting.
| ) | ||
|
|
||
| # mass flow through mining process | ||
| outputs["raw_ore"] = processed_ore_production * ref_raw_ore / ref_pellets |
There was a problem hiding this comment.
is raw ore consumed?
There was a problem hiding this comment.
Yes, raw ore, crushed ore, and concentrated ore are consumed within the model, that's why they follow a different convention instead of {commodity}_out. The tailings are available hence following convention
jmartin4u
left a comment
There was a problem hiding this comment.
Approved, assuming nothing is amiss after the push I just made. All I did was make a new example to ensure that the new mine model is running and fitting into the overall plant structure fairly seamlessly, and it is! Costs are not changing significantly between the old 21_iron_example/iron_dri and new 21_iron_examples/iron_dri_nrri examples. If you want to avoid "example bloat" feel free to get rid of whichever one you choose - the main thing I wanted to see was that overall levelized cost of iron ore/iron/steel was still getting calculated correctly, and that there was nothing else across H2I that needed to change to make it fit in. The ONE change that still needed to be made was to the pipe model - it needed to have diesel added as a transportable commodity, in units of galUS
NRRI Iron Range Mines Models
This is a different iron mine performance model that splits the mining process into separate steps that tracks the material flows and energy/fuel use on a per process step basis.
The cost model is based on SEC reports for iron ore pellets and crude ore extracted.
Section 1: Type of Contribution
Section 2: Draft PR Checklist
TODO:
Type of Reviewer Feedback Requested (on Draft PR)
Structural feedback:
Implementation feedback:
Other feedback:
Section 3: General PR Checklist
docs/files are up-to-date, or added when necessaryCHANGELOG.md"A complete thought. [PR XYZ]((https://github.com/NatLabRockies/H2Integrate/pull/XYZ)", where
XYZshould be replaced with the actual number.Section 4: Related Issues
Section 5: Impacted Areas of the Software
Section 5.1: New Files
iron/nrri_ore/perf_coeffs.csv: has the mass flow per process, energy and fuel consumption per process for each mine.iron/nrri_ore/cost_coeffs.csv: has the SEC costs for each mine per LT.iron/nrri_iron_mine.pyNRRIIronMinePerformanceComponent: This is aflexibledispatched iron mine. It calculates the energy and fuel consumption on a per process basis. The system can only be curtailed but does not take airon_ore_command_valueas an input.NRRIIronMineCostComponent: This uses thecost_coeffs.csvto pull in the SEC costs per LT for each mine. It only outputsOpExsince it assumesCapExis annualized into the operational costs.Section 5.2: Modified Files
docs: Updated theiron_mine.mdto include new models. @jmartin4u maybe you can update this based on conversations with NRRI? @kbrunik updated with sources from Kim's docsupported_models.pyconverters/iron/martin_ore/: Renamed to `converters/iron/simple_ore/converters/iron/martin_transport/: Renamed to `converters/iron/simple_transport/converters/iron/test/test_martin_mine.py: Renamed toconverters/iron/test/test_simple_mine.pyconverters/iron/martin_mine_cost_model.py: Renamed toconverters/iron/simple_mine_cost_model.pyconverters/iron/martin_mine_perf_model.py: Renamed toconverters/iron/simple_mine_perf_model.pySection 6: Additional Supporting Information
Section 7: Test Results, if applicable
Section 8 (Optional): New Model Checklist
docs/developer_guide/coding_guidelines.mdattrsclass to define theConfigto load in attributes for the modelBaseConfigorCostModelBaseConfiginitialize()method,setup()method,compute()methodCostModelBaseClass__init__.pyfile to ensure it is properly imported and used insupported_models.pysupported_models.pycreate_financial_modelinh2integrate_model.pytest_all_examples.pydocs/user_guide/model_overview.mddocs/section<model_name>.mdis added to the_toc.ymlgenerate_class_hierarchy.pyto update the class hierarchy diagram indocs/developer_guide/class_structure.md