Skip to content
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

Logical flow #71

Merged
merged 43 commits into from
Jan 15, 2025
Merged

Logical flow #71

merged 43 commits into from
Jan 15, 2025

Conversation

samgdotson
Copy link
Collaborator

@samgdotson samgdotson commented Dec 23, 2024

Summary of changes

This PR adds a new dispatch model to osier called LogicDispatchModel that follows a hierarchy of rules to dispatch energy. It is much faster (~4x speedup) than the original DispatchModel but it is myopic and therefore results may be sub-optimal. Other benefits include:

  • No solver required, commercial or otherwise. Thus osier is more self-contained.
  • Easier to understand since the mathematics have been reduced to a sequence of comparisons.
  • Drop-in replacement for DispatchModel. No change to syntax.
  • Enables parallelization of the genetic algorithm driving the CapacityExpansion model.

Note

There are a couple of new tests for new functionality. These tests should exceed the minimum code coverage required by the pyproject.toml file (75%). More tests are desired, but not necessary, here.

Edit: Closes #54 by changing MW*hr to MWh.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Checklist for Reviewers

Reviewers should use this link to get to the
Review Checklist before they begin their review.

Copy link
Contributor

@LukeSeifert LukeSeifert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I just had a couple comments on some docstrings and the MWh usage. I also had some trouble running the tests myself. The tests fail with installation method 1 (from source) and method 2 for me after I cloned your repo and switched to the logical_flow branch. Do you know what I need to do to test this version of osier?

osier/models/logic_dispatch.py Show resolved Hide resolved
osier/models/logic_dispatch.py Outdated Show resolved Hide resolved
osier/models/logic_dispatch.py Show resolved Hide resolved
osier/tech_library.py Outdated Show resolved Hide resolved
osier/technology.py Outdated Show resolved Hide resolved
osier/technology.py Show resolved Hide resolved
osier/technology.py Show resolved Hide resolved
tests/test_technology.py Show resolved Hide resolved
tests/test_technology.py Show resolved Hide resolved
tests/test_technology.py Show resolved Hide resolved
Copy link
Contributor

@yardasol yardasol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @samgdotson, great work on this PR. A couple items

  • Docpages have not been updated to hold the new classes
  • Line-items (see comments)

Additionally, it's not clear to me where the no-solver change is, nor where parallelization of the genetic algorithm is enabled. If you could describe these changes in a bit more detail that'd be wonderful!

osier/models/capacity_expansion.py Outdated Show resolved Hide resolved
osier/models/logic_dispatch.py Show resolved Hide resolved
osier/models/model.py Show resolved Hide resolved
osier/tech_library.py Outdated Show resolved Hide resolved
osier/technology.py Outdated Show resolved Hide resolved
osier/technology.py Outdated Show resolved Hide resolved
osier/utils.py Show resolved Hide resolved
osier/utils.py Show resolved Hide resolved
osier/technology.py Show resolved Hide resolved
osier/technology.py Show resolved Hide resolved
Copy link
Collaborator Author

@samgdotson samgdotson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reviews @yardasol and @LukeSeifert!

osier/models/logic_dispatch.py Show resolved Hide resolved
osier/models/model.py Show resolved Hide resolved
@samgdotson
Copy link
Collaborator Author

@yardasol, just addressing your comments/questions:

  1. There is no parallelization algorithm in this PR. The PR enables parallelization.
  2. The "no-solver" change lives in CapacityExpansion where I added a flag called model_engine.

The docs should be updated, now. Also, I'm not sure why the GH actions are failing, but it's not anything related to the code.

Copy link
Contributor

@LukeSeifert LukeSeifert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I made a comment on one docstring that has the parameters missing, everything else seems good to me

osier/technology.py Show resolved Hide resolved
@samgdotson
Copy link
Collaborator Author

@LukeSeifert @yardasol if you're satisfied with the changes feel free to merge (in spite of the failing checks -- I think it is a GH issue).

@samgdotson
Copy link
Collaborator Author

Okay, I think the issue is because Mambaforge is no longer being maintained, so the installer is looking for a link that doesn't go anywhere, anymore. See this release.

Working on a fix.

@samgdotson
Copy link
Collaborator Author

@yardasol, checks pass, now. Are you satisfied with the changes?

@LukeSeifert, regarding testing locally, I can't say for sure what's wrong because I haven't seen the error message. However, I speculate that the errors might be because you haven't "installed" the branch. Try creating an editable installation (as described in the contributing guide) with

python -m pip install -e .[doc]

This way, you can switch between branches at will and the "installed version" of osier will follow code changes (including changing branches).

Copy link
Contributor

@yardasol yardasol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good changed @samgdotson. I found one little snag but other than that we're good to go!

osier/models/capacity_expansion.py Show resolved Hide resolved
osier/models/capacity_expansion.py Show resolved Hide resolved
osier/technology.py Outdated Show resolved Hide resolved
@samgdotson
Copy link
Collaborator Author

Thanks @yardasol! Feel free to merge whenever!

@yardasol yardasol merged commit 6434619 into arfc:main Jan 15, 2025
13 checks passed
@samgdotson samgdotson deleted the logical_flow branch January 15, 2025 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Comp:Core This issue has to do with the main bulk of the code or document. (methods, main content) Difficulty:2-Challenging This issue may be complex or require specialized skills. Priority:1-Critical This is the highest priority (i.e. it is blocking other work or facing a deadline). Status:5-In Review This issue has been handled, and the solution is being reviewed. (probably as a PR) Type:Feature New feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

simplify units in osier
4 participants