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.

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)
• Update storage/control models to use the naming "commodity_rate_units" and "commodity_amount_units" instead of "commodity_units" or "commodity_storage_units"
• Update finance models (this will also change test result values). 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"
• Update feedstock component to use commodity and commodity rate units
  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
  • nitrogen ASU model

• Update/add outputs to storage models

  • hydrogen
  • battery

• 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

• 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

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

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
  • method1: What and why something was changed in one sentence or less.

Section 4.2: Modified Files

• path/to/file.extension
  • method1: What and why something was changed in one sentence or less.

Section 5: Additional Supporting Information

Future development (in other PRs) 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

Section 6: Test Results, if applicable

[RHammond2]

I like the idea of enforcing that some of these parameters need to exist to simplify setup and later usage, but there should be an actual enforcement method that requires the commodity, commodity_rate_units, commodity_amount_units, and the later to-be-determined attribues to be defined. Without an actual enforcement it will let the error crop up in a likely confusing place once it's been long enough since this feature has been implemented.

Something along these lines would likely work to avoid getting too fancy with Python's abstract base classes. i would definitely make the error message a bit more descriptive during implementation, but this keeps it to a single line for this example.

class PerformanceModelBaseClass():  # will need om.ExplicitComponent, but am skipping for the sake of a replicable example
    def __new__(cls, *args, **kwargs):
        required = ("commodity", "commodity_rate_units", "commodity_amount_units")
        missing = [el for el in required if not hasattr(cls, el)]
        if missing:
            missing = ", ".join(missing)
            raise NotImplementedError(f"{cls.__name__} missing the following attributes: {missing}")
        return super().__new__(cls)

In this case, we could define the following two sample subclasses.

class WindPerformanceBaseClass(PerformanceModelBaseClass):
    commodity = "electricity"

class OtherPerformanceBaseClass(PerformanceModelBaseClass):
    commodity = "electricity"
    commodity_rate_units = "kwh"
    commodity_amount_units = "kw"

When called, OtherPerformaceBaseClass() will successfully initialize, but when attempting to create an instance of WindPerformanceBaseClass, you'll be met with the error.

NotImplementedError: WindPerformanceBaseClass missing the following attributes: commodity_rate_units, commodity_amount_units

The only catch is that if base classes themselves aren't tested for basic setup, then this will cascade to the class being instantiated.

class Wind(WindPerformanceBaseClass):
    def __init__():
        pass

>>> Wind()
NotImplementedError: Wind missing the following attributes: commodity_rate_units, commodity_amount_units

[elenya-grant]

Thanks Rob! I think that's a great idea and will play around with implementation! Thanks for the detailed suggestion

I implemented this and I get not implemented errors on the models that do inherit this model. Perhaps this has something to do with __new__() being called before setup() (the necessary attributes are set in the setup() methods). Any suggestions?

My current thought is to put this check in the setup() method of the baseclass or in the set_outputs() method of the baseclass.

[RHammond2]

If the values are being set in setup(), then that would be an issue because __new__() is looking for the existence of the attributes at initialization. I recommended moving them out of setup and into the top-level simply because they will remain the same for any instance of the class, and are effectively constants within it.

One aspect that I didn't mention is that this should be paired with basic tests for each child base class to ensure they're setup correctly. This way, we can pinpoint where the failure would come from before playing around with examples.

[elenya-grant]

For the time being, I've added the check in the set_outputs() method which is called in setup() (which is always called when the model is using in OpenMDAO). I'm no expert on OpenMDAO, but my impression is that initialize() is nearly equivalent to __init__(), but the "options" cannot be accessed in this method (they are only declared). Also - this initialize method is now done in the PerformanceModelBaseClass since it's the same for all performance models. I played around with adding in an __init__() method but I'm not sure if this would have any unexpected impacts on behavior. Aka - I wonder if OpenMDAO has some quirks that make the om.ExplicitComponent classes behave a bit differently than normal python classes. But - I'd be curious if @johnjasa has any insight here.

I think that adding the warning to the setup() method (which is what I've done for now) does address the concern about enforcing these attributes to be set in the performance models.

I can also add in some tests for child base classes. I pushed up a new test for the solar performance baseclass.

[johnjasa]

I like this dialogue and appreciate the clear suggestions you've been giving, Rob, along with your implementation of it, Elenya!

I'm glad you brought up the OpenMDAO-related portions of this, Elenya. I think that we may be butting up against something: generally in OpenMDAO models, Component classes don't have attributes directly on the class. Instead, they use the options dict as detailed here and mentioned by you. This is due to the special way components and groups are constructed at the OM level, using the initialize and setup methods as you mention.

In this PR, we're introducing attrs on the classes themselves, not on the config object that the class owns. That config object is generally a great receptacle for many of these attrs, or at least that's how we've been using them in H2I. Have you explored using the config object as the storer for these attrs? Or maybe using the OM options dict itself? I haven't fully ideated that, but if you've already riffed on it, would love to hear your thoughts.

[RHammond2]

@elenya-grant as long as the error is getting raised at class initialization that should work for me!

[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]

No I didn't wind up using total_commodity_produced for my reverse sizing method. I create new max_commodity_capacity variables that go "backwards" - e.g. max_hydrogen_capacity is an output of technology that consumes hydrogen as a feedstock, an input of a tech that produces hydrogen as a commodity. But this could have been leftover code from me not properly cleaning PR #198 before it got merged, not sure...

[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?