Ban implicit broadcasting over year dimension#841
Merged
Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
This reverts commit b83550a.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the “ban implicit broadcasting” work to the year dimension by adding an explicit broadcast_years helper, updating core model code to calculate key quantities year-by-year (not 2 years at once), and tightening dispatch input validation to avoid hidden shape errors.
Changes:
- Add
broadcast_yearsand raise an explicit error when xarray tries to implicitly broadcast alongyear. - Refactor sector market variable calculations to compute supply/consumption/costs per-year and then concatenate results.
- Update multiple call sites and tests to explicitly broadcast/shape data rather than relying on implicit year expansion.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_regressions.py | Updates regression expectations to use explicit year broadcasting. |
| tests/test_quantities.py | Attempts to force single-year capacity in tests (but introduces a fixture recursion issue). |
| tests/test_dispatch.py | Updates dispatch test inputs to remove year dimension from prices (but introduces a fixture recursion issue). |
| tests/test_demand_share.py | Explicitly broadcasts technologies over capacity years for matching-market logic. |
| tests/test_constraints.py | Removes year selection after maximum_production by selecting capacity year before the call. |
| tests/conftest.py | Refactors technology fixture broadcasting across region/year (but leaves fixed_outputs without year). |
| src/muse/utilities.py | Adds broadcast_years helper to make year expansion explicit. |
| src/muse/timeslices.py | Ensures timeslice fractions are explicitly broadcast over year when needed. |
| src/muse/sectors/sector.py | Computes supply/consumption/costs year-by-year instead of operating on a 2-year block. |
| src/muse/regressions.py | Uses explicit year broadcasting for regression coefficients (but can call broadcast_years on scalars). |
| src/muse/readers/toml.py | Explicitly broadcasts timeslice shares over years when combining with consumption. |
| src/muse/investments.py | Makes retirement profile comparisons explicitly year-broadcast-safe. |
| src/muse/examples.py | Broadcasts per-asset technologies across years for example matching-market generation. |
| src/muse/dispatch.py | Replaces asserts with dimension checks; simplifies merit-order dispatch to operate on per-year inputs. |
| src/muse/costs.py | Makes discounting/lifetime conversion explicit over year via broadcast_years. |
| src/muse/agents/factories.py | Explicitly broadcasts agent shares across capacity years when scaling capacity. |
| src/muse/agents/agent.py | Ensures investments are explicitly broadcast over year before applying retirement profile. |
| src/muse/main.py | Raises a clear error on implicit broadcasting across the year dimension during debug/tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows on from #840
This involved slightly more changes, but still doable. In doing this I realised it would be better to calculate dispatch/consumption/prices year-by-year rather than multiple (2) years at a time. In doing so I think I've fixed up an error with the prices calculation in the first year (using investment year technology parameters). Not that these prices are used, but this this has relevance for #835