openems icon indicating copy to clipboard operation
openems copied to clipboard

fix(near equals power solver): Invalid behavior for limits

Open parapluplu opened this issue 1 month ago • 4 comments

This fixes an error where a charge limit was set for a single ess inside a cluster. The power solver disrespected this, and set a value outside of the limit. For details see unit test part #4 of `testNearEqualDistribution in io.openems.edge.ess.core/test/io/openems/edge/ess/core/power/PowerComponentTest.java. Without this fix, this test fails and charges higher than the allowed -1900.

Testing: Unit tests, also tested in a setup with a power plant with 12 ESS in total stacked in two layers of ESS clusters. First layer is an cluster of 3 Edge2Edge ESS, second layer is a Cluster of 4 EssGenericManagedSymmetric each.

parapluplu avatar Nov 27 '25 09:11 parapluplu

Codecov Report

:x: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

:x: Your patch check has failed because the patch coverage (50.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #3447      +/-   ##
=============================================
+ Coverage      59.62%   59.63%   +0.01%     
  Complexity       112      112              
=============================================
  Files           2894     2894              
  Lines         124658   124644      -14     
  Branches        9343     9342       -1     
=============================================
- Hits           74318    74313       -5     
+ Misses         47522    47520       -2     
+ Partials        2818     2811       -7     
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Nov 27 '25 09:11 codecov[bot]

Could somebody explain to me we codecov/patch is failing with a 50% diff target? I've basically removed code and added tests. The contrary should happen and 50% is waaaaay out of line

parapluplu avatar Nov 27 '25 10:11 parapluplu

Could somebody explain to me we codecov/patch is failing with a 50% diff target? I've basically removed code and added tests. The contrary should happen and 50% is waaaaay out of line

TBH i never understood codecov here :D

Sn0w3y avatar Nov 27 '25 14:11 Sn0w3y

I'd like to publish another merge request, that improves the null handling of the power solver in general. However, as it builds on top of this change, I would like to make my life easier and merge this one before publishing the other one. Also they are different kind of changes, why I want to keep them seperated as commits. Is there any chance the change in this merge request gets merged?

parapluplu avatar Dec 17 '25 00:12 parapluplu

https://github.com/OpenEMS/openems/commit/701d7b839d8ed6023bddfb3d4f15f4a8bc8f50ba

This was merged 7 hours ago with the back port merge and creates conflicts for this pull request. So if this merge request does not get any further traction I will drop this topic and will keep the changes and follow up changes in our internal repo since this now creates significant overhead.

I've also changed the null handling to optionals, which makes it much more explicit and enforces correct handling whenever touching the values. Seems like Fenecon decided to go for a approach with using a constant instead of Optionals https://github.com/OpenEMS/openems/blob/develop/io.openems.edge.ess.core/src/io/openems/edge/ess/core/power/optimizers/KeepAllNearEqual.java#L27

I would prefer a more modern approach provided by java

parapluplu avatar Dec 17 '25 17:12 parapluplu