Skip to content

Comments

Enforce minimum service factor limits#325

Merged
alexdewar merged 5 commits intodevelopfrom
enforce_minimum_service_factor_limits
Jun 5, 2024
Merged

Enforce minimum service factor limits#325
alexdewar merged 5 commits intodevelopfrom
enforce_minimum_service_factor_limits

Conversation

@alexdewar
Copy link
Collaborator

Description

I've fixed #320 by introducing a check in the minimum_service constraint function that all minimum service factors are >=0 and <= 1 and I've also amended the spurious tests accordingly.

The only thing is that I'm wondering whether this is a bit late to raise an error. Should the check be in some earlier parsing step (either in addition or instead)?

While I was reading through the docstring for muse.constraints I noticed some typos and fixed those while I was at it.

Fixes #320.

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

@alexdewar alexdewar requested review from dalonsoa and tsmbland June 5, 2024 10:45
@codecov
Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 71.09%. Comparing base (89754e6) to head (c7a0101).

Files Patch % Lines
src/muse/constraints.py 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #325      +/-   ##
===========================================
- Coverage    71.11%   71.09%   -0.02%     
===========================================
  Files           44       44              
  Lines         5809     5812       +3     
  Branches      1147     1148       +1     
===========================================
+ Hits          4131     4132       +1     
- Misses        1358     1359       +1     
- Partials       320      321       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right check, wrong place :)

if np.all(technologies["minimum_service_factor"] == 0):
return None

min_service_factor = technologies["minimum_service_factor"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the right place to make this check is in the readers, when the technodata is read and sort of validated, somewhere around here:

data = check_utilization_not_all_zero(data, filename)

This is validating the technodata timeslices file, but probably it should also be applied to the normal technodata file, in the previous function in the same module.

@alexdewar alexdewar merged commit 31268b0 into develop Jun 5, 2024
@alexdewar alexdewar deleted the enforce_minimum_service_factor_limits branch June 5, 2024 14:34
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.

Invalid values for minimum service factor in test_minimum_service_factor

2 participants