search

EnergySystemsModellingLab / MUSE_OS

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

Fix incorrect use of LCOE and ill-defined example #304

Closed dalonsoa closed 1 month ago

dalonsoa commented 1 month ago

Description

This PR fixes a couple of problems:

  1. The LCOE now returns a 0 for those timeslices where the utilization factor is zero - which would result in an infinity LCOE as a result. Fixes #246 .
  2. The default_timeslice model was ill-defined, with wrong values for the MinimumServiceFactor, which must be between 0 and 1, inclusive.
  3. Fixes a weird bug that was reveled when changing the above two. It's unclear why things were not failing before, but I suspect that another, unrelated error was causing the associated tests to pass. The real problem is that the tests that were supposed to check this are really more integration tests that actual unit tests, so a lot of things were going on.

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.

Key checklist

Further checks

ahawkes commented 1 month ago

Thanks this looks good to me. Is there another issue required to fix the input data on the time slice problem?

From: Diego Alonso Álvarez @.> Sent: 14 May 2024 16:01 To: EnergySystemsModellingLab/MUSE_OS @.> Cc: Hawkes, Adam D @.>; Review requested @.> Subject: Re: [EnergySystemsModellingLab/MUSE_OS] Fix iincorrect use of LCOE and ill-defined example (PR #304)

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 requested your review on: #304https://github.com/EnergySystemsModellingLab/MUSE_OS/pull/304 Fix iincorrect use of LCOE and ill-defined example.

— Reply to this email directly, view it on GitHubhttps://github.com/EnergySystemsModellingLab/MUSE_OS/pull/304#event-12803952715, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC37JLNRXLHAXB2IUZBTBSDZCIRKZAVCNFSM6AAAAABHWLWTUCVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSHAYDGOJVGI3TCNI. You are receiving this because your review was requested.Message ID: @.***>

alexdewar commented 1 month ago

One regression test is failing for all the runners... I'm guessing we're getting a different result now because the calculation has changed @dalonsoa?

dalonsoa commented 1 month ago

Thanks this looks good to me. Is there another issue required to fix the input data on the time slice problem?

As far as I can tell, that's all. The results seems to make sense and once the minimum service issue is solved and meaningful values are provided for it, the calculation completes even with some utilization factors equal to zero, which I think was the next main barrier.

I would suggest we merge this, create a release and you try to use it to check that it produces sound results.

ahawkes commented 1 month ago

OK thanks. Maybe Tom could look at the impact on the tutorials, to try to see if it looks like the result make sense?

Note, however, that the overall approach is still lacking despite this fix. This is because LCOE (and presumably other objectives) is calculated based on maximum utilisation. But when solved, actual utilisation could be a lot lower (e.g. the technology might be used only in 1 time slice), thus increasing LCOE substantially, potentially (likely!) making the technology choice wrong ex-post of the dispatch.

Rgds, Adam

From: Diego Alonso Álvarez @.> Sent: 15 May 2024 05:52 To: EnergySystemsModellingLab/MUSE_OS @.> Cc: Hawkes, Adam D @.>; Review requested @.> Subject: Re: [EnergySystemsModellingLab/MUSE_OS] Fix incorrect use of LCOE and ill-defined example (PR #304)

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.

Thanks this looks good to me. Is there another issue required to fix the input data on the time slice problem?

As far as I can tell, that's all. The results seems to make sense and once the minimum service issue is solved and meaningful values are provided for it, the calculation completes even with some utilization factors equal to zero, which I think was the next main barrier.

I would suggest we merge this, create a release and you try to use it to check that it produces sound results.

— Reply to this email directly, view it on GitHubhttps://github.com/EnergySystemsModellingLab/MUSE_OS/pull/304#issuecomment-2111579522, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC37JLKK7Z3HYXCR5UUCUWLZCLSXLAVCNFSM6AAAAABHWLWTUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJRGU3TSNJSGI. You are receiving this because your review was requested.Message ID: @.***>

dalonsoa commented 1 month ago

Note, however, that the overall approach is still lacking despite this fix. This is because LCOE (and presumably other objectives) is calculated based on maximum utilisation. But when solved, actual utilisation could be a lot lower (e.g. the technology might be used only in 1 time slice), thus increasing LCOE substantially, potentially (likely!) making the technology choice wrong ex-post of the dispatch.

I'm quite unsure on how to implement this since actual utilisation won't be known until dispatch, which happens after the investment. Do you have any suggestion on how this could be done?

ahawkes commented 1 month ago

I think it would require substantial changes, and indeed there is no perfect solution that I am aware of. In MUSE 1.0, suggest we leave this as-is for now, as a known issue. In MUSE 2.0 I am looking at it, and I think we can get close to a solution that considers actual post-dispatch LCOE at time of investment, but it will never be perfect (much like the real world, which in some senses is what MUSE was developed to do!) and will also probably present some new issues.

From: Diego Alonso Álvarez @.> Sent: 15 May 2024 10:40 To: EnergySystemsModellingLab/MUSE_OS @.> Cc: Hawkes, Adam D @.>; Review requested @.> Subject: Re: [EnergySystemsModellingLab/MUSE_OS] Fix incorrect use of LCOE and ill-defined example (PR #304)

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.

Note, however, that the overall approach is still lacking despite this fix. This is because LCOE (and presumably other objectives) is calculated based on maximum utilisation. But when solved, actual utilisation could be a lot lower (e.g. the technology might be used only in 1 time slice), thus increasing LCOE substantially, potentially (likely!) making the technology choice wrong ex-post of the dispatch.

I'm quite unsure on how to implement this since actual utilisation won't be known until dispatch, which happens after the investment. Do you have any suggestion on how this could be done?

— Reply to this email directly, view it on GitHubhttps://github.com/EnergySystemsModellingLab/MUSE_OS/pull/304#issuecomment-2112045140, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC37JLOK7AGFEWGNLJSDUGLZCMURZAVCNFSM6AAAAABHWLWTUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJSGA2DKMJUGA. You are receiving this because your review was requested.Message ID: @.***>

dalonsoa commented 1 month ago

@tsmbland it might be simpler if you review this and we merge it, and then you update your branch with develop. Splitting it into smaller bits is not going to help was you still need to bring all changes to your branch, anyway.