Skip to content

fix solve time for logical dispatch model #73

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.

Sign up for GitHub

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

Jump to bottom
Merged
katyhuff merged 3 commits into from
Jul 3, 2025
Merged

fix solve time for logical dispatch model #73

katyhuff merged 3 commits into from
Jul 3, 2025

Conversation

@samgdotson
Copy link
Collaborator

@samgdotson samgdotson commented Jun 29, 2025

Summary of changes

This PR adds addresses a bug that caused a major slowdown for the logical dispatch model when using it with the CapacityExpansion problem. I fixed the issue by removing the call for synchronize_units within the base Model class since users can do this manually and CapacityExpansion will do it by default. It should only happen once, but was instead happening for every evaluation.

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.

Associated Issues and PRs

  • Issue: #

Associated Developers

  • Dev: @

Checklist for Reviewers

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

@samgdotson samgdotson requested a review from yardasol June 29, 2025 20:45
@samgdotson samgdotson self-assigned this Jun 29, 2025
@samgdotson samgdotson added 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:Bug Something is wrong or broken. This issue or PR is related to a bug in code. labels Jun 29, 2025
@samgdotson samgdotson requested a review from SamarAbdelkawy July 2, 2025 21:46
@samgdotson
Copy link
Collaborator Author

samgdotson commented Jul 2, 2025 •
edited
Loading

@SamarAbdelkawy, I'm adding you as a reviewer to this pull request since you might be interested in using Osier at some point!

katyhuff
katyhuff approved these changes Jul 3, 2025
View reviewed changes
Copy link
Member

@katyhuff katyhuff 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, reviewed together.

@katyhuff katyhuff merged commit 9383aaf into arfc:main Jul 3, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@katyhuff katyhuff katyhuff approved these changes

@yardasol yardasol Awaiting requested review from yardasol

@SamarAbdelkawy SamarAbdelkawy Awaiting requested review from SamarAbdelkawy

Assignees

@samgdotson samgdotson

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:Bug Something is wrong or broken. This issue or PR is related to a bug in code.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@samgdotson @katyhuff