Skip to content

Conversation

@tsmbland
Copy link
Collaborator

@tsmbland tsmbland commented Oct 24, 2024

Description

This PR fixes a few more bugs caused by automatic broadcasting.

I identified these using the approach suggested by @dalonsoa here, which works beautifully. It essentially turns off automatic broadcasting (I've limited this to the timeslice dimension), and throws an error message when broadcasting would be required to perform the intended operation. I've implemented this in a separate PR/branch (#530). I then ran the code in that branch and went through every place where it complained about automatic broadcasting, and was able to spot a few more places where convert_timeslice should be applied to split quantities over the timeslice dimension (rather than automatic broadcasting as was being done previously).

I then went back and applied most of these changes here. Long run we'll want to merge #530 to properly turn off automatic broadcasting (rather than just fixing errors like I'm doing here). However, there are lots of places where it ends up complaining about benign things, and it will still take some work to make it completely happy. Right now I just want to fix these bugs as quickly as possible. (#530 is also way ahead of this one and has a completely different timeslices module which isn't ready yet.)

Summary of the changes:

  • Fix an error in the ALCOE function where fuel costs and env costs for the whole year were applied to each timeslice. This ends up effecting commodity prices
  • Fix a similar error in the emission_cost objective
  • Fix a similar error in the emission function, where emissions for the whole year were being applied to every timeslice. This mostly has implications for models using the carbon budget, as emissions will now be much lower than before. I've updated the carbon budget tutorial to reflect this
  • Fix a similar error in gross_margin with the calculation of consumption and production costs

There's at least one more bug of this type remaining (in the consumption function), but I'm going to tackle that in a separate PR as it has much larger implications
Fixes # (issue)

Type of change

  • 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 changed the title Broadcast errors Fix more broadcasting errors Oct 24, 2024
@tsmbland tsmbland marked this pull request as ready for review October 24, 2024 14:29
@tsmbland tsmbland requested a review from dalonsoa October 24, 2024 14:29
@tsmbland
Copy link
Collaborator Author

Actually I'm mistaken about the emission function. It was correct before. Going to change it back.

Same thing with the error in consumption that I alluded to

@tsmbland
Copy link
Collaborator Author

Closing this and opening #534 instead

@tsmbland tsmbland closed this Oct 24, 2024
@tsmbland tsmbland deleted the broadcast_errors branch October 24, 2024 15:57
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.

2 participants