Add new PEM and NG solid oxide fuel cell models - #794
Conversation
elenya-grant
left a comment
There was a problem hiding this comment.
hi Gen! I really only looked at the PEM_h2_fuel_cell.py method but wanted to give some preliminary feedback. My main thoughts are:
- I think keeping track of what's calculated at the cell level vs stack level vs the system level is a bit confusing in the
compute()method and thecalc_currentfunction. I think some extra in-line comments would be helpful for improving clarity. - I think that there are some mis-matches on units (kg/hr vs kg/dt)
- I think the looping in the
compute()method could be removed. - using
Ifor current andJfor current density would help me follow the logic!
Overall - this is a great start! I haven't dug into the doc page or the examples or tests yet but would love to on a re-review! Let me know if you want to chat about any of my comments sometime! Happy to hop on a call!
| # fuel_cell_efficiency_hhv: float = field(validator=range_val(0, 1)) | ||
|
|
||
|
|
||
| def calc_current(power_ref, cell_area, n_cells, stack_number): |
There was a problem hiding this comment.
I may not be fully appreciating what this function does, but I don't think the power_curve part is needed, power = current*voltage. I have some ideas on possible revisions but add those in later.
There was a problem hiding this comment.
I think what's partially confusing about this method is what scale it should be on. like power_ref is the power to the entire system. Then power is divided among stacks. Every cell in the stack has the same current and voltage. below is what I tried to do instead and it gave similar results as your method:
def calc_current_refactor(power_to_system, cell_area, n_cells, n_stacks):
# note: power_to_system should be in W
J_curve = np.array([
0.0356,
0.05413333,
0.0796,
0.11366667,
0.244,
0.454,
]) # A/cm2
voltage_curve = np.array([
0.987,
0.936,
0.884,
0.838,
0.786,
0.736])
# function to calculate voltage from current density
V_coefs = np.polyfit(J_curve, voltage_curve, 5)
V_J_curve = np.poly1d(V_coefs)
# function to calculate current density from power
I_curve = J_curve*cell_area
P_curve = I_curve*(n_cells*voltage_curve)
J_coefs = np.polyfit(P_curve, J_curve, 5)
J_P_curve = np.poly1d(J_coefs)
power_per_stack = power_to_system/n_stacks
stack_current_density = J_P_curve(power_per_stack)
stack_current = stack_current_density*cell_area
cell_voltage = V_J_curve(stack_current_density)
return stack_current, cell_voltage, V_J_curve| I_cell = max(power_I_curve(power_density), 0) | ||
| V_cell = V_I_curve(I_cell) | ||
| I_cell = I_cell * cell_area |
There was a problem hiding this comment.
I may need to work out the units for myself to check the logic here but I think that if you're doing this:
I_cell = I_cell * cell_areathat makes me think that current_curve is actually current density (A/cm2), even though the comment says A. could you clarify whether the I_cell being returned is current or current density and similar question for current_curve?
There was a problem hiding this comment.
You're right, the current curve is for current density! I'll clarify that!
| ) | ||
|
|
||
| # Calculate hydrogen and oxygen consumed | ||
| H2_consumed_rate = ((I_cell * self.N_series * self.M_H2) / (2.0 * self.f_c)) * ( |
There was a problem hiding this comment.
I think it may be nice to split this line up a bit:
h2_consumption_rate_stack =(n_cells * I_cell) / (2 * faradays) # mol/s
h2_consumption_rate_system = n_stacks*h2_consumption_rate_stack*H2_MW/1e3 # kg/s
h2_consumed = h2_consumption_rate_system*self.dt| # is n_cells = N_series? | ||
| self.N_series = 1 | ||
| self.stack_size = self.config.system_capacity_kw / self.config.n_stacks | ||
| self.cell_active_area = 400 # [cm^2] from Battelle (https://www.energy.gov/sites/prod/files/2018/02/f49/fcto_battelle_mfg_cost_analysis_1%20_to_25kw_pp_chp_fc_systems_jan2017_0.pdf) |
There was a problem hiding this comment.
should cell active area be recalculated so that stack_size = n_cells*cell_active_area*power_dens?
Co-authored-by: elenya-grant <116225007+elenya-grant@users.noreply.github.com>
elenya-grant
left a comment
There was a problem hiding this comment.
Howdy Gen! This is looking great! I had a few small comments but nothing too serious. I looked primarily at the SO_NG_fuel_cell - but some of the comments are relevant to both models. I'm curious if theres any way to utilize some base-class here, but it's tough since they're in different folders. Anyway - I think it's looking good and would be happy to chat through any comments if ya want! thanks!
I just want to double-check some units-stuff (it never hurts to have multiple eyes on units conversions) but once I do that - I'll approve this! I imagine that it just needs a few small updates, some commented out code removed, then it's good to go!
| ) | ||
|
|
||
| self.add_output( | ||
| "carbon_dioxide_out", |
There was a problem hiding this comment.
some of the carbon capture technologies (in converters/co2/marine/) use co2 in the variable names instead of carbon_dioxide. I think that it'd be nice for us to use the same naming convention for the same commodities. Anyone have a preference on carbon_dioxide vs co2? (@kbrunik)
There was a problem hiding this comment.
I've updated to co2_out for now!
| M_CO2 = CO2_MW / 1000 # Molar mass of CO2 in kg/mol | ||
|
|
||
| # calculate max input and output | ||
| inputs["oxygen_in"] # kg/h |
There was a problem hiding this comment.
reminder to remove this line
| I_stack_from_ng = (ng_in_kg_per_s * 8 * faraday) / (M_CH4 * self.config.n_stacks) | ||
| I_stack_from_o2 = (o2_in_kg_per_s * 4 * faraday) / (M_O2 * self.config.n_stacks) |
There was a problem hiding this comment.
Could you add a comment noting what the 8 and 4 here represent? I'm assuming its number of moles or electrons transferred?
| self.add_input( | ||
| "system_capacity", | ||
| val=self.config.system_capacity_kw, | ||
| units="kW", | ||
| desc="Capacity of the solid oxide natural gas fuel cell system", | ||
| ) |
There was a problem hiding this comment.
what if this input was rated_electricity_production? as the output from the performance model? I don't think thats strictly necessary but the rated_electricity_production output follows some standard naming convention whereas the system_capacity input is semi-unique to this performance model (aka - we haven't standardized input design variable naming convention for converters so it could be nice to use the output names)
There was a problem hiding this comment.
I don't think I'm executing this correctly, so let's chat about this more!
| outputs["water_out"] = h2o_generated | ||
| outputs["carbon_dioxide_out"] = co2_generated |
There was a problem hiding this comment.
the fuel cell primary output commodity is electricity, but also produces water and carbon dioxide. If someone wanted to (for whatever reason) calculate the LCO of the carbon dioxide or the water production from the fuel cell, the fuel cell model would also have to output rated_carbon_dioxide_production and rated_water_production (those outputs may be nice to add just in case)
| self.add_input( | ||
| "system_capacity", | ||
| val=self.config.system_capacity_kw, | ||
| units="kW", | ||
| desc="Capacity of the h2 fuel cell system", | ||
| ) |
There was a problem hiding this comment.
similar comment here as the SO NG fuel cell cost model about changing this input to rated_electricity_production instead?
Add new PEM and NG fuel cell models
This PR adds two new fuel cell models to H2I:
Each of these models include an I-V curve and basic reaction modeling to determine the feedstocks consumed.
Section 1: Type of Contribution
Section 2: Draft PR Checklist
TODO:
Type of Reviewer Feedback Requested (on Draft PR)
Structural feedback:
There is some potential overlap here with the steam natural gas reforming model that already exists in the hydrogen folder. In the SO NG fuel cell model, the fuel cell itself still uses a reaction of hydrogen and oxygen, but the natural gas is put through a steam reforming process before the deconstructed H_2 and CO are fed into the fuel cell. The addition of other things in the fuel cell input other than pure hydrogen does affect the fuel cell performance, so I think it's a worthwhile addition to the code on its own. In the model added, natural gas goes into the model, and electricity, water, and carbon dioxide come out.
Let me know your thoughts on possible overlap issues!
Implementation feedback:
Are these models where you would expect them to be in the code?
Thoughts on Solid Oxide Natural Gas model (SONG)? Should it be NGSO (natural gas solid oxide) instead?
Other feedback:
Are these being worked on/needed in/for other projects?
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
path/to/file.extensionmethod1: What and why something was changed in one sentence or less.Section 5.2: Modified Files
path/to/file.extensionmethod1: What and why something was changed in one sentence or less.Section 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()methodCostModelBaseClasssupported_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