EnergySystemsModellingLab / MUSE_OS

Welcome to the MUSE-OS repository
https://muse-os.readthedocs.io/en/latest/
GNU General Public License v3.0
22 stars 9 forks source link

Fix the `supply` function in `quantities.py` to impose minimum constraint #335

Open tsmbland opened 3 months ago

tsmbland commented 3 months ago

I've just found why the discrepancy.

  1. The sector output, taking the values right out of the market, gives the right values as market information is calculated based on the constrains (so figure on the right is correct).
  2. The MCA information, surprisingly and I don't know why, is obtained by recalculating everything. In this line, it limits the supply so it is not higher than the maximum production, but it does not impose any minimum constrain but to fulfil the demand, and therefore the minimum service constrained is not honoured.

In summary, the calculation for the MCAMetric_Supply.csv is wrong - and I would presume that any other calculation that uses this supply function will also be wrong, as no minimum constrains are imposed but to fulfill the demand.

_Originally posted by @dalonsoa in https://github.com/EnergySystemsModellingLab/MUSE_OS/discussions/321#discussioncomment-9749169_

tsmbland commented 3 months ago

@dalonsoa Are you working on this, or would you like me to look into it?

ahawkes commented 3 months ago

Another high priority issue!


From: Tom Bland @.> Sent: Wednesday, June 12, 2024 12:54:57 PM To: EnergySystemsModellingLab/MUSE_OS @.> Cc: Subscribed @.***> Subject: Re: [EnergySystemsModellingLab/MUSE_OS] Fix the supply function in quantities.py to impose minimum constraint (Issue #335)

This email from @.*** originates from outside Imperial. Do not click on links and attachments unless you recognise the sender. If you trust the sender, add them to your safe senders listhttps://spam.ic.ac.uk/SpamConsole/Senders.aspx to disable email stamping for this address.

@dalonsoahttps://github.com/dalonsoa Are you working on this, or would you like me to look into it?

— Reply to this email directly, view it on GitHubhttps://github.com/EnergySystemsModellingLab/MUSE_OS/issues/335#issuecomment-2162818626, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC37JLIGORIHSE52OSEVK5DZHAZJDAVCNFSM6AAAAABJGGIT4GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSHAYTQNRSGY. You are receiving this because you are subscribed to this thread.Message ID: @.***>

dalonsoa commented 3 months ago

@tsmbland , nope, so all yours!

dalonsoa commented 3 months ago

So, I've been looking into this and, after digging in the exciting world of decorators and registries, it turns out this function is used whenever dispatch_production = 'share' is chosen for a sector, which is most of the times. So:

  1. We cannot simply get rid of this function.
  2. The function has been doing things wrong all along, as it does not respect the minimum service factor.

So the fix passes through updating the function such that we also calculate a minimum_production and impose it here.

ahawkes commented 3 months ago

Ok. I think we need to understand the intention of dispatch_production = share. This is because it might be possible for a user to input conflicting information. E.g. if dispatch is set to share means that the dispatch must follow the demand profile shape, but a minimum service constraint is also imposed that does not allow this, then an error in input data should be flagged. However I’m not entirely sure that that is the intention of dispatch_production = share. Is there another option that is something like “merit order”?


From: Diego Alonso Álvarez @.> Sent: Friday, June 21, 2024 9:28:10 AM To: EnergySystemsModellingLab/MUSE_OS @.> Cc: Hawkes, Adam D @.>; Comment @.> Subject: Re: [EnergySystemsModellingLab/MUSE_OS] Fix the supply function in quantities.py to impose minimum constraint (Issue #335)

This email from @.*** originates from outside Imperial. Do not click on links and attachments unless you recognise the sender. If you trust the sender, add them to your safe senders listhttps://spam.ic.ac.uk/SpamConsole/Senders.aspx to disable email stamping for this address.

So, I've been looking into this and, after digging in the exciting world of decorators and registries, it turns out this function is used whenever dispatch_production = 'share' is chosen for a sector, which is most of the times. So:

  1. We cannot simply get rid of this function.
  2. The function has been doing things wrong all along, as it does not respect the minimum service factor.

So the fix passes through updating the function such that we also calculate a minimum_production and impose it herehttps://github.com/EnergySystemsModellingLab/MUSE_OS/blob/d331f46c13a70889ca093492ea441848aec14a79/src/muse/quantities.py#L92.

— Reply to this email directly, view it on GitHubhttps://github.com/EnergySystemsModellingLab/MUSE_OS/issues/335#issuecomment-2182265563, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC37JLPUFHEXDQFOQSM77YDZIPPZVAVCNFSM6AAAAABJGGIT4GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBSGI3DKNJWGM. You are receiving this because you commented.Message ID: @.***>

dalonsoa commented 3 months ago

E.g. if dispatch is set to share means that the dispatch must follow the demand profile shape, but a minimum service constraint is also imposed that does not allow this

The maximum production (which includes the utilisation factor) can also prevent this from happening, so also using the minimum service constraint will not lead to a new situation that is not potentially happening already.

However I’m not entirely sure that that is the intention of dispatch_production = share. Is there another option that is something like “merit order”?

Ok, as it is often the case, this is an undocumented/wrongly documented feature of MUSE. If you go to the input file documentation (to this section) it explains what the investment_production and the dispatch_production are. But the valid options for either case (which are the same) are not correct. These options can be found in the code itself (and the API documentation), everything with the register_production decorator. The current options are: maximum, share, match and costed. I'm not sure if any of them do what you want.

ahawkes commented 3 months ago

Wow. Don't all of the 4 options potentially (likely!) contradict the dispatch as computed in the call to highs that we've discussed previously? Why are they all calculated again here? Am I missing something?

dalonsoa commented 3 months ago

I've created a PR (#368) to close this issue and, after seeing what files were modified, I think I know why there're two routes for calculating stuff (to be checked): The individual dispatch functions are only really used by the ad-hoc solver, not the scipy solver, which takes the output of the solver as the dispatch, as it should since it contains all the constrains. The ad-hoc solver needs to calculate everything manually, and hence this functions.

This is not really documented, as far as I can tell, which leads to incorrect use of the input parameters, normally irrelevant as they will not affect the result or it will result in contradictory outputs. The MCAMetric-related outputs used the quantities calculated with the ad-hoc solver, which are not necessarily the same than the scipy solver.

ahawkes commented 3 months ago

But have we solved the fundamental issue? It's not OK to have contradictory outputs, right? I do not understand why this "ad-hoc" solver exists. Why do we have this output reported in two different places?

dalonsoa commented 3 months ago

@ahawkes , the adhoc solver has always existed, coming from the time of the original MUSE but using xarray and the more modern architecture setup by Mayeul, and indeed it is still the default, although none of the models or examples use it. There has never been a suggestion or discussion to drop it, so there it is.

The fundamental issue is, mainly, one of documentation.

ahawkes commented 3 months ago

OK, but is it simple enough to change the default to spicy? And make sure that this conflicting result is not provided to the user when using spicy? Because it's understandably very confusing.

I think probably worth dropping the ad-hoc as it does not appear to respect input data, but we should ask @sgiarols of any known implications. Any thoughts Sara?