Problem managing the violations in N

[pgambier]

A bug is to be fixed in the code for managing situations with violations in N (pull request #33).

The problem was found in class ConstraintsModifier in methods getNewUpperLimit and getNewLowerLimit.

Here is a description of the problem for the method getNewUpperLimit (the problem is symmetrical for getNewLowerLimit).

private float getNewUpperLimit(LimitViolation violation, float margin) {
    float newLimit = 9999;
    if ( config.isInAreaOfInterest(violation, network) ) {
        float increment = (float) ((violation.getLimit() == 0) 
             ? Math.ceil(violation.getValue()*100) 
             : Math.ceil((violation.getValue()-violation.getLimit())*100/violation.getLimit()));
        increment += margin;
        newLimit = (violation.getLimit() == 0) 
             ? (increment/100) 
             : (violation.getLimit()+(violation.getLimit()*increment/100));
    }
    return newLimit;
}

Major problem : the ceil method is used on line 273 (or line 6 of the excerpt above) on a value which is often inferior to 1 (current or voltage value is usually close to the limit when there is a violation in N) therefore the increment value is likely to be equal to one.
Because the value of violation limits can be high (around 2000 A in 400kV in France), with increment = 1, newLimit ends up being much higher than the initial limit with margin = 0 which is hardly understandable and dangerous for the user.

Minor problem : using the intermediary variable increment for the calculation makes the code difficult to understand.

A suggestion is to remove the increment variable and directly calculate new limit without rounding anything
new limit = load flow value in N + (load flow value in N * margin in %)
or rounding as the last step of the calculation
new limit = ceil(load flow value in N + (load flow value in N * margin in %))

[mathbagu]

Written in java, it could be something like this:

private float getNewUpperLimit(LimitViolation violation, float margin) {
    float newLimit = 9999;
    if ( config.isInAreaOfInterest(violation, network) ) {
        newLimit = violation.getValue() * (1.0f + margin);
    }
    return newLimit;
}

 private float getNewLowerLimit(LimitViolation violation, float margin) {
     float newLimit = -9999;
     if ( config.isInAreaOfInterest(violation, network) ) {
         newLimit = violation.getValue() * (1.0f - margin);
     }
     return newLimit;
 }

I think that the checkModifiedNetworkLimits method should also be modified.

[CBiasuzzi]

just a small change to the new code ( since the margin is given in pct ..... )

private float getNewUpperLimit(LimitViolation violation, float margin) {
    float newLimit = 9999;
    if ( config.isInAreaOfInterest(violation, network) ) {
        newLimit = violation.getValue() * (1.0f + margin / 100.0f );
    }
    return newLimit;
}

private float getNewLowerLimit(LimitViolation violation, float margin) {
     float newLimit = -9999;
     if ( config.isInAreaOfInterest(violation, network) ) {
         newLimit = violation.getValue() * (1.0f - margin / 100.0f );
     }
     return newLimit;
 }