Skip to content

Comments

Correctly use the minimum service factor#388

Merged
dalonsoa merged 2 commits intoeven-better-tomlfrom
min_service_factor
Jul 2, 2024
Merged

Correctly use the minimum service factor#388
dalonsoa merged 2 commits intoeven-better-tomlfrom
min_service_factor

Conversation

@dalonsoa
Copy link
Collaborator

@dalonsoa dalonsoa commented Jul 2, 2024

Description

As ti was being used as a fraction of the utilisation factor rather as a fraction of 100%.

Fixes #375

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.

  • 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

@dalonsoa dalonsoa changed the base branch from develop to even-better-toml July 2, 2024 05:21
@dalonsoa dalonsoa requested review from alexdewar and tsmbland July 2, 2024 05:21
@codecov
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.37%. Comparing base (baf1efb) to head (d981c14).

Additional details and impacted files
@@                 Coverage Diff                  @@
##           even-better-toml     #388      +/-   ##
====================================================
+ Coverage             71.34%   71.37%   +0.03%     
====================================================
  Files                    44       44              
  Lines                  5890     5890              
  Branches               1162     1162              
====================================================
+ Hits                   4202     4204       +2     
+ Misses                 1366     1365       -1     
+ Partials                322      321       -1     

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

Copy link
Collaborator

@alexdewar alexdewar left a comment

Choose a reason for hiding this comment

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

LGTM, other than the one question I have.

)
capacity = convert_timeslice(
techs.fixed_outputs * techs.utilization_factor * techs.minimum_service_factor,
techs.fixed_outputs * techs.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.

Where does the division by the maximum output happen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It doesn't: the minimum service factor is the result of that division, that is why it is a fraction! fixed_outputs would be the absolute maximum and you multiply it by utilization_factor or the minimum_service_factor to recover the upper and lower limits of the production.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I seeeee

Copy link
Collaborator

@tsmbland tsmbland left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@dalonsoa dalonsoa merged commit b69aca9 into even-better-toml Jul 2, 2024
@tsmbland
Copy link
Collaborator

tsmbland commented Jul 2, 2024

Did you mean to merge this into the even-better-toml branch?

@dalonsoa dalonsoa deleted the min_service_factor branch July 2, 2024 09:15
@dalonsoa
Copy link
Collaborator Author

dalonsoa commented Jul 2, 2024

No, my mistake :(

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.

Minimum service factor is applied incorrectly

3 participants