coin-or / Rehearse

Algebraic modeling library in C++ for linear optimization solvers
MIT License
43 stars 15 forks source link

Does CelExpression has a leak #15

Open Stephocke opened 6 years ago

Stephocke commented 6 years ago

Hi,

it seems that the class CelExpression has a leak within the overloaded operators =.

I add a minimal example: A loop with one iteration requires 0.4 MB Ram, 200,000 iterations require over 200 MB Ram

#include "CelNumVar.h"
#include <iostream>

int main()
{
    CelNumVar x1("x1");
    CelNumVar x2("x2");
    CelExpression expr;

    for (int i = 0; i < 200000; i++)
        expr = 4 <= x1 + x2;

    expr.display();

    std::cin.get();
}

I have tried the following, which solve the issue. But i have only tested the sample code above.

CelExpression & CelExpression::operator = (CelExpression &expression) 
{
        // avoid self assignment
    if (this == &expression)
        return *this;

    if (right && right->deletable)
        delete right;
    if (left && left->deletable)
        delete left;

    this->node_type = CelExpression::NODE_PROXY;
    this->left = &expression;
    this->right = NULL;

    return *this;
}
Stephocke commented 6 years ago

A comand like "model.addConstraint( 1 * x1 + x2 == 18 );" does also leak because the "tempory" LinExpression (NODE_OP_EQ) is deletable itself but you could not include "delete this;" within the destructor, because you never know, if the current LinExpression is just a part of a nested Expression.

It seems that a complete new implemtation of the friend operators are required. Gurobi for instance just return a copy of the resulting LinExpression and trust on the return value optimization. Furthermore they suggest to use "void operator+=" to build up large expressions to avoid to much copy operations within the friend operator functions (quadratic runtimes depending on the current expression size). They distinguish between an LinExpression and constraints, that means there are no "==", "<=" and ">=" operator functions for LinExpressions. During the construction of a constraint all duplicated terms (variable) are merged.

http://www.gurobi.com/documentation/8.0/refman/py_linexpr.html