Skip to content

Modify demand_limiting_capacity constraint#458

Merged
tsmbland merged 5 commits intodevelopfrom
dlc
Aug 20, 2024
Merged

Modify demand_limiting_capacity constraint#458
tsmbland merged 5 commits intodevelopfrom
dlc

Conversation

@tsmbland
Copy link
Copy Markdown
Collaborator

@tsmbland tsmbland commented Aug 16, 2024

Description

This modifies the demand-limiting capacity constraint to address the issue described in #457. I've added lots of comments to the code so hopefully everything is relatively clear, but happy to have a chat about it if not.

None of the example models are affected, as this only affects certain technologies like refineries that have multiple end-use commodities, but I've tested this with Alaa's Qatar model and the results look reasonable.

Fixes #457

Type of change

Please add a line in the relevant section of
CHANGELOG.md to
document the change (include PR #) - note reverse order of PR #s.

  • New feature (non-breaking change which adds functionality)
  • Optimization (non-breaking, back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Key checklist

  • All tests pass: $ python -m pytest
  • The documentation builds and looks OK: $ python -m sphinx -b html docs docs/build

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@tsmbland tsmbland marked this pull request as ready for review August 19, 2024 09:22
@tsmbland tsmbland requested a review from dalonsoa August 19, 2024 09:23
@tsmbland tsmbland changed the title Modify demand_limiting_capacity constraint Modify demand_limiting_capacity constraint Aug 19, 2024
Copy link
Copy Markdown
Collaborator

@dalonsoa dalonsoa 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 but very hard to understand the underlying logic - specially in the long term. I suggest you extract the new code as a standalone function and add some dedicated test - can even be a doctest, so it is included with the documentation - illustrating what is going on here and what problem is solving.

@tsmbland
Copy link
Copy Markdown
Collaborator Author

@dalonsoa I've extracted the code to a new function and created a doctest, as suggested, so hopefully it's clearer now.

Also, things started to fail with python 3.12 due to the numpy 2.1 release, so I've had to pin numpy to 2.0 for now.

@tsmbland tsmbland requested a review from dalonsoa August 20, 2024 12:20
Copy link
Copy Markdown
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

I wish all MUSE code had this detailed explanations...

@tsmbland tsmbland merged commit 413bd31 into develop Aug 20, 2024
@tsmbland tsmbland deleted the dlc branch August 20, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Problem with demand_limiting_capacity constraint for technologies that have multiple end-use commodities [BUG]

2 participants