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

Invalid values for minimum service factor in `test_minimum_service_factor` #320

Closed tsmbland closed 6 days ago

tsmbland commented 1 month ago

As established in #304, MinimumServiceFactor should be between 0 and 1. However, the test_minimum_service_factor test uses invalid values >1, and still passes

Ideally we shouldn't be in a position where the model can be run with invalid values, however at the very least this test should be modified to use valid values.

It's also worth making sure this test is checking the correct thing. It seems to be comparing the absolute value of supply against the MinimumServiceFactor, however my understanding is that MinimumServiceFactor relates to relative supply compared to the theoretical maximum (hence why it needs to be between 0 and 1). Or maybe I'm misunderstanding something...

ahawkes commented 1 month ago

Your understanding is correct.

It should fail with a helpful error if a user specifies a value greater than 1. And it should pass with values between 0 and 1. I guess if one wanted to take things further the maximum service factor (which I think we’re calling utilisation - a bit of a misnomer) should be greater than or equal to the minimum service factor, and between 0 and 1.

I’m sure you are - but just to double check you’re accounting for time slice length here - minimum/maximum energy is also dependent on time slice length of course.

A


From: Tom Bland @.> Sent: Friday, May 31, 2024 2:03:43 PM To: EnergySystemsModellingLab/MUSE_OS @.> Cc: Subscribed @.***> Subject: [EnergySystemsModellingLab/MUSE_OS] Invalid values for minimum service factor in test_minimum_service_factor (Issue #320)

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.

As established in #304https://github.com/EnergySystemsModellingLab/MUSE_OS/pull/304, MinimumServiceFactor should be between 0 and 1. However, the test_minimum_service_factor test uses invalid values >1, and still passes

Ideally we shouldn't be in a position where the model can be run with invalid values, however at the very least this test should be modified to use valid values.

It's also worth making sure this test is checking the correct thing. It seems to be comparing the absolute value of supply against the MinimumServiceFactor, however my understanding is that MinimumServiceFactor relates to relative supply compared to the theoretical maximum (hence why it needs to be between 0 and 1). Or maybe I'm misunderstanding something...

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

alexdewar commented 3 weeks ago

Reopening because I reverted the PR