Feature/gsye 901

[hannesdiedrich]

Reason for the proposed changes

Please describe what we want to achieve and why.

Proposed changes

INTEGRATION_TESTS_BRANCH=master
GSY_FRAMEWORK_BRANCH=feature/GSYE-901
SCM_ENGINE_BRANCH=master

[codecov]

Codecov Report

❌ Patch coverage is 83.01887% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.12%. Comparing base (54e7bef) to head (de39098).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1911      +/-   ##
==========================================
- Coverage   71.12%   71.12%   -0.01%     
==========================================
  Files         149      149              
  Lines       14122    14166      +44     
  Branches     1872     1879       +7     
==========================================
+ Hits        10044    10075      +31     
- Misses       3548     3555       +7     
- Partials      530      536       +6     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[spyrostz]

I see that the PLR limit is still 1 and not 1.05 though, if this is the expected behavior then I would remove the comment since it looks invalid

[hannesdiedrich]

Ah sorry, this comment is from an intermediate version. Will remove.

[spyrostz]

It works, but I would prefer to not use indexes in this function. For example, something like this would equally work without using list indexes, and it is more concise (note that you can use the Walrus operator in the dict comprehension, but I left it out due to also not being a huge fan of it):

PLR_dict = {
    q / (self._model["Qref"] * CAPFT): q 
    for q in Q_solutions 
    if 0 <= q / (self._model["Qref"] * CAPFT) <= 1
}
if not PLR_dict:
    log.error("IndividualCOPModel: No physically feasible PLR solutions. %s", PLR_dict)
    return None
return PLR_dict[max(PLR_dict)]

[hannesdiedrich]

Thanks! Will implement your suggestion, because I do not mind either way.
Is there a reason to prefer dictionaries here besides personal preference?
Thanks for omitting the Walrus operator :-)

[spyrostz]

Is there a reason to prefer dictionaries here besides personal preference?

It is mostly personal preference and trying to avoid index-based calculations when possible. IMO index-based calculations are very error prone and worth to replace with dictionaries or other calculation types whenever possible.

[hannesdiedrich]

Understood, thanks!

[spyrostz]

Some small comments, LGTM once resolved

[spyrostz]

LGTM