Standardize performance model outputs

[elenya-grant]

Standardize performance model outputs

Introducing a baseclass for performance models to standardize outputs. The standardized outputs are:

• commodity_out in units of commodity_rate_units: commodity output profile of length n_timesteps
• total_commodity_produced in units of commodity_amount_units: sum of commodity produced over simulation (adjusted for timestep if necessary)
• annual_commodity_produced in units of commodity_amount_units/yr: annual commodity production adjusted for a year-long simulation, shape is equal to plant_life.
• replacement_schedule in units of unitless: the percent of the capacity that is replaced per year. Defaults to an array of zeros of length plant_life.
• capacity_factor in units of unitless: the capacity factor of a system as a fraction. Has a shape equal to plant_life.
• rated_commodity_production in units of commodity_rate_units: rated production capacity of the converter. Used to calculate capacity_factor
• operational_life in units of yr: operational life of the technology, defaults to plant life. Tentatively planning on adding it in.

The attributes that each performance model needs to define prior to calling the setup() method in the PerformanceModelBaseClass are: commodity, commodity_rate_units and commodity_amount_units. For example, the wind performance models in the setup method would do:

class WindPerformanceBaseClass(PerformanceModelBaseClass):
    def setup(self):
        self.commodity = "electricity"
        self.commodity_rate_units = "kW"
        self.commodity_amount_units = "kW*h"
        super().setup()

An electrolyzer performance model would do:

class ElectrolyzerPerformanceBaseClass(PerformanceModelBaseClass):
    def setup(self):
        self.commodity = "hydrogen"
        self.commodity_rate_units = "kg/h"
        self.commodity_amount_units = "kg"
        super().setup()

This would resolve some issues around hard-coded tech-specific logic in H2IntegrateModel and the profast finance models, and issues relating to unit convention for varying simulation lengths or timesteps. To keep this PR small, this PR will focus on adding these standardized outputs to all the converter models, with the goal of preventing changes in test values. This PR also introduces a lot of TODO notes, which will be addressed in follow-on PRs because most of those changes would likely result in test value changes. Aka - some TODOs will not be removed in this PR.

This PR is primarily focuses on adding outputs to performance models and adding basic tests to ensure that all the outputs are set (not testing the value that they're set to, just that they're not zero). To ensure that the planned changes that may result in different test values or larger framework changes to core models (which this PR lays the foundation for) are highlighted, these are planned to be done in future PRs so that all the additions in this PR do not distract from those changes.

Out of scope for this PR is:

• standardizing performance model inputs (not planned as part of this larger effort)
• adding in technology-specific unit tests to test specific output values (captured in Issue Tests that are needed, should be moved, or should be reformatted #481)
• making technology models flexible to varying timesteps or simulation lengths
• removing existing outputs of performance models that are equivalent to the new outputs introduced with the PerformanceModelBaseClass

The follow-on PRs will be:

• Update electrolyzer outputs to be per-year (this will change a lot of test values that are testing hydrogen production, LCOH, or any LCO values of components downstream of the electrolyzer). This could be part of the PR for Issue Update ProFAST finance models to use utilization instead of capacity (1 of 3) #485
• Update storage/control models to use the naming "commodity_rate_units" and "commodity_amount_units" instead of "commodity_units" or "commodity_storage_units". This is noted in Issue Update remaining models to use commodity_rate_units and commodity_amount_units (2/3) #486
• Update finance models (this will also change test result values). This is noted in Issue Update ProFAST finance models to use utilization instead of capacity (1 of 3) #485. The updates are:
  • use capacity factor as the long term utilization
  • use rated commodity production as the capacity
• Remove tech-specific logic in H2IntegrateModel, remove ElectricitySumComp, remove logic in "generic_summer.py". This would be part of the PR to resolve issue Update remaining models to use commodity_rate_units and commodity_amount_units (2/3) #486
• Update feedstock component to use commodity and commodity rate units (This would be part of the PR to resolve issue Update remaining models to use commodity_rate_units and commodity_amount_units (2/3) #486
• Remove tech-specific or commodity-specific hard-coded logic in core models, this is noted in Issue Reduce tech-specific or commodity-specific logic in H2IntegrateModel and finance models (3/3) #488

These PRs can get started once this PR is merged in.

Benefits of this PR are:

• standardized output naming can reduce tech-specific hard-coded logic in H2IntegrateModel, finance models, etc
• standardized output naming may also reduce the number of places in the code that a new commodity type has to be added when a tech with a new commodity type is added
• standardized usage of rate units vs amount units

Section 1: Type of Contribution

• Feature Enhancement
  • Framework
  • New Model
  • Updated Model
  • Tools/Utilities
  • Other (please describe):
• Bug Fix
• Documentation Update
• CI Changes
• Other (please describe):

Section 2: Draft PR Checklist

• Open draft PR
• Describe the feature that will be added
• Fill out TODO list steps
• Describe requested feedback from reviewers on draft PR
• Complete Section 7: New Model Checklist (if applicable)

TODO:

• Update/add outputs to converter models and add basic unit-tests that outputs are set

  • wind
  • solar
  • water_power
  • ammonia
    • simple ammonia
    • ammonia synloop
  • co2 (in progress/partially done, has remaining todos for removing duplicate outputs)
    • doc
    • oae
  • grid
  • hopp
  • hydrogen (electrolyzer) (in progress/partially done, has remaining todos)
  • hydrogen (geologic)
  • iron
    • iron mine (old - should be deleted)
    • standalone iron mine (Martin mine)
    • iron wrapper (old - should be deleted)
    • iron plant (old - should be deleted)
    • iron dri
  • methanol
    • smr
    • co2h
  • natural gas
  • steel
    • steel.py (used in Ex 1)
    • electric arc funance (old - should be deleted)
    • steel eaf plant
  • water (desal)
  • paper mill in Example 6 (future PR)
  • nitrogen ASU model

• Update/add outputs to storage models

  • [-] hydrogen (N/A)
  • battery
  • simple_generic_storage (future PR, Issue Update remaining models to use commodity_rate_units and commodity_amount_units (2/3) #486)
  • simple_storage_autosizing (future PR, Issue Update remaining models to use commodity_rate_units and commodity_amount_units (2/3) #486)

• [-] Update/add outputs to feedstock model and fix units (future PR)

• [-] Update combiners and splitters (as necessary, mostly done in future PR)

• [-] Update ProFAST finance models to use capacity factor as utilization and rated_commodity_production as capacity (make as a separate PR)

• [-] Update AdjustedCapexOpexComp if needed.

• Update/fix tests as needed

• Update post-processing functions as necessary

• [-] Update documentation on adding a new technology (future PR)

• Add unit-tests to check that all outputs set via PerformanceModelBaseClass are given values in the compute() method of parent classes.

Type of Reviewer Feedback Requested (on Draft PR)

Structural feedback:

• thoughts on standardized output naming convention?

Implementation feedback:

• Changes to the wind, solar, water power, and electrolyzer converters have already been done and can be reviewed now for reviewers to provide high-level feedback.

Other feedback:

• do we want to assume a year is 8760 hours or use what OpenMDAO assumes is a year (8765.812776 hours/year)?

Section 3: General PR Checklist

• PR description thoroughly describes the new feature, bug fix, etc.
• Added tests for new functionality or bug fixes
• Tests pass (If not, and this is expected, please elaborate in the Section 6: Test Results)
• Documentation
  • Docstrings are up-to-date
  • Related docs/ files are up-to-date, or added when necessary
  • Documentation has been rebuilt successfully
  • [-] Examples have been updated (if applicable)
• CHANGELOG.md has been updated to describe the changes made in this PR

Section 3: Related Issues

Existing Related Issues

Units for varying timesteps and simulation lengths: Issue #244, #204, and #387 (may be partially resolved with this PR)
Standardized naming conventions: Issue #223 (this would be partially resolved with this PR)
Remove dependence on name of the technologies in H2I: Issue #374 (would be partially/fully resolved in this PR)
Issue about converter baseclass (somewhat related): Issue #231

New issues made

• Issue Tests that are needed, should be moved, or should be reformatted #481 (tests that are missing or need to be reformatted)
• Issue Update ProFAST finance models to use utilization instead of capacity (1 of 3) #485 (for follow-on PR)
• Issue Update remaining models to use commodity_rate_units and commodity_amount_units (2/3) #486 (for follow-on PR)
• Issue Updates needed to adding_a_new_technology.md #487 (update docs on how to add a new tech)
• Issue Reduce tech-specific or commodity-specific logic in H2IntegrateModel and finance models (3/3) #488 (for follow-on PR)

Section 4: Impacted Areas of the Software

Section 4.1: New Files

• h2integrate/converters/hydrogen/test/test_pem_electrolyzer_performance.py
• h2integrate/converters/steel/test/test_simple_steel.py
• h2integrate/converters/methanol/test/test_methanol.py
• h2integrate/converters/water_power/test/test_hydro_power.py

Section 4.2: Modified Files

Most converter performance models or performance baseclasses and some variable names in subtests

Section 5: Additional Supporting Information

Future development (in other PRs than the ones noted above) that could build on this framework are:

• standardize outputs for techs that produce multiple commodities
• standardize inputs and outputs for techs that require/use feedstocks
• updates to specific technology models to account for varying timestep (dt) or simulation length (n_timesteps)

Section 6: Test Results, if applicable

No test values changed!

[genevievestarke]

I think this looks great! I like the standardizations proposed and I can see how it's implemented in the individual technologies!

[genevievestarke]

Did this method go somewhere else?

[elenya-grant]

its not used - this is somewhat leftover from early on where each tech was going to have its own performance, cost, and finance baseclass and models. I imagine a handful of unused baseclasses may be removed in this PR

[jmartin4u]

This a nice big undertaking and should make interconnectivity between models a lot smoother moving forward, thanks for taking it on. I don't want to throw any wrenches in the gears of implementation except to say it would be nice a have the commodity_rate_units editable in a tech_config parameter, but that could wait for a future feature add.

[jmartin4u]

If capacity_factor is an annual calculation I think we should make that clear in the variable name - call it annual_capacity_factor instead?

[elenya-grant]

I think that's a valid name choice, I'd want to get the rest of the teams opinion on it before changing it. Could also be done in a separate PR

[bayc]

Really like these changes! I agree with the others that it brings a much needed uniformity to the models. Left a few comments throughout, some building on others. Thanks @elenya-grant !

[bayc]

These type of tests are useful! Wondering if we also have regression tests for this tech? I see the test below is checking some values, but wondering about checking the values of these standardized outputs. Is that for a future PR? Is the main issue the tedious nature of updating values when they change? Definitely have a lot of thoughts on testing.

[elenya-grant]

Some models did not have regression tests and when a future PR removes "duplicated" outputs, then I'll update existing unit tests to use the new naming convention where applicable. I think that adding in tech-specific unit tests are not part of this effort and should be done when new technologies are added. I made Issue #481 to summarize my thoughts on testing that arose from this, some models did not have any unit tests but I added this basic test to ensure that I didn't forget to set one of the new outputs.

[bayc]

Seconding @johnjasa suggestion of looking at having these in the config class.

[elenya-grant]

My thoughts are that most performance models are specific to a technology, which inherently has at least one output commodity and the calculations are done with respect to the units defined. For example, the desalination model uses volume for the water inputs/outputs whereas another model may require water mass. Therefore, I think it makes sense for commodities and commodity units to be inherent to the class rather than the configuration class. I think it makes sense to tie the units to these models to the performance class rather than the configuration class since the units and commodity are specific to the calculations done in the performance class.

Additionally, when I picture getting rid of tech-specific or commodity-specific hard-coded logic in H2IntegrateModel - I'd imagine that it may be useful to get the attribute from the technology class (tech.commodity) rather than its configuration class tech.config.commodity

[elenya-grant]

Since this was mentioned by multiple folks, I will say that I think this should be discussed with the larger team at some point but could be done after the planned development related to this PR is complete.

[elenya-grant]

@johnjasa

I realized that to remove some tech-specific logic in H2IntegrateModel, that it would require setting the attributes commodity, commodity_rate_units, and commodity_amount_units in the initialize() method instead of the setup() method. What are you thoughts on this:

class WindPerformanceBaseClass(PerformanceModelBaseClass):
    def initialize(self):
        super().initialize()
        self.commodity = "electricity"
        self.commodity_rate_units = "kW"
        self.commodity_amount_units = "kW*h"

instead of this:

    def setup(self):
        self.commodity = "electricity"
        self.commodity_rate_units = "kW"
        self.commodity_amount_units = "kW*h"       
        super().setup()

This would also be okay in tech-neutral technologies because most of those would have to define the commodity and units in the tech_config which we could also access within H2IntegrateModel

[jaredthomas68]

Looking good. I still have a few outstanding comments I would like to get resolved prior to merging this in.

[jaredthomas68]

Looks good and ready to merge. Well done @elenya-grant!