A major change to Coek internals

1. Removing void* references to Var/Param reference types

2. Using shared pointer references instead, to ensure proper memory
mangement.

3. MANY follow-on changes were required to implement this change.

4. A key change is the reversion of IndexVector to std::vector<int>.
The simple memory management used in IndexVector is not sufficient to
handle the potential for dynamic alloction/decallocation of
IndexVectors.

NOTE: These changes will likely incur a performance penalty in coek
(especialy #4).  But the earlier code design did not work properly when
using mapped variables and parameters.

Author: whart222

Diff for: lib/coek/coek/api/expression.cpp

@@ -282,6 +282,20 @@ bool Variable::is_binary() const { return repn->binary; }
 
 bool Variable::is_integer() const { return repn->integer; }
 
+Variable Variable::expand()
+{
+#ifdef COEK_WITH_COMPACT_MODEL
+    auto var = Variable().
+        lower( this->lower_expression().expand() ).
+        upper( this->upper_expression().expand() ).
+        value( this->value_expression().expand() ).
+        within( this->within() );
+    return var;
+#else
+    return *this;
+#endif
+}
+
 std::ostream& operator<<(std::ostream& ostr, const Variable& arg)
 {
     write_expr(arg.repn, ostr);

Diff for: lib/coek/coek/api/expression.hpp

@@ -268,6 +268,9 @@ class Variable {
     /** \returns \c false because this is not a constant expression */
     bool is_constant() const { return false; }
 
+    /** \returns an expanded Variable */
+    Variable expand();
+
     /**
      * \name Stream operator
      *

Diff for: lib/coek/coek/api/indexed_container.defs.hpp

@@ -12,7 +12,9 @@ class IndexedComponentRepn_multiarray : public IndexedComponentRepn<TYPE> {
    public:
     IndexedComponentRepn_multiarray(size_t n) : IndexedComponentRepn<TYPE>(1), shape({n})
     {
-        this->cache.resize((n + 1) * 2);
+        #ifdef CUSTOM_INDEXVECTOR
+        this->cache.resize(2 * (n + 1) * 2);
+        #endif
     }
 
     IndexedComponentRepn_multiarray(const std::vector<size_t>& _shape)
@@ -21,7 +23,9 @@ class IndexedComponentRepn_multiarray : public IndexedComponentRepn<TYPE> {
         size_t _size = 1;
         for (auto n : shape)
             _size *= n;
-        this->cache.resize((_size + 1) * (_shape.size() + 1));
+        #ifdef CUSTOM_INDEXVECTOR
+        this->cache.resize(2 * (_size + 1) * (_shape.size() + 1));
+        #endif
     }
 
     IndexedComponentRepn_multiarray(const std::initializer_list<size_t>& _shape)
@@ -30,7 +34,9 @@ class IndexedComponentRepn_multiarray : public IndexedComponentRepn<TYPE> {
         size_t _size = 1;
         for (auto n : shape)
             _size *= n;
-        this->cache.resize((_size + 1) * (_shape.size() + 1));
+        #ifdef CUSTOM_INDEXVECTOR
+        this->cache.resize(2 * (_size + 1) * (_shape.size() + 1));
+        #endif
     }
 
     virtual ~IndexedComponentRepn_multiarray() {}
@@ -88,7 +94,9 @@ class IndexedComponentRepn_setindex : public IndexedComponentRepn<TYPE> {
     IndexedComponentRepn_setindex(ConcreteSet& _arg)
         : IndexedComponentRepn<TYPE>(_arg.dim()), concrete_set(_arg)
     {
-        this->cache.resize((this->dim() + 1) * (_arg.size() + 1));
+        #ifdef CUSTOM_INDEXVECTOR
+        this->cache.resize(2 * (this->dim() + 1) * (_arg.size() + 1));
+        #endif
         this->tmp.resize(this->dim());
     }
 
@@ -119,8 +127,12 @@ void IndexedComponentRepn_setindex<TYPE>::generate_names()
         return;
 
     size_t _dim = this->dim();
+    #ifdef CUSTOM_INDEXVECTOR
     std::vector<int> x_data(_dim);
     IndexVector x(&(x_data[0]), _dim);
+    #else
+    IndexVector x(_dim);
+    #endif
     for (auto& indices : concrete_set) {
         for (size_t j = 0; j < _dim; j++)
             x[j] = indices[j];
@@ -209,7 +221,11 @@ TYPE& IndexedComponent_Map<TYPE>::index(const IndexVector& args)
 
     auto curr = this->repn->value.find(args);
     if (curr == this->repn->value.end()) {
+        #ifdef CUSTOM_INDEXVECTOR
         auto _args = this->repn->cache.clone(args);
+        #else
+        auto& _args = args;
+        #endif
         TYPE tmp;
         auto& res = this->repn->value[_args] = tmp;
         return res;

Diff for: lib/coek/coek/api/indexed_container.hpp

@@ -24,7 +24,9 @@ typedef std::variant<int, expr_pointer_t> refarg_types;
 template <class TYPE>
 class IndexedComponentRepn {
    public:
+    #ifdef CUSTOM_INDEXVECTOR
     IndexVectorCache cache;
+    #endif
     std::map<IndexVector, TYPE> value;
     size_t _dim;
     std::string _name;
@@ -38,7 +40,11 @@ class IndexedComponentRepn {
 
     void resize_index_vectors(IndexVector& tmp, std::vector<refarg_types>& reftmp)
     {
-        tmp = cache.alloc(dim());
+        #ifdef CUSTOM_INDEXVECTOR
+        tmp = cache.alloc(2*dim());
+        #else
+        tmp.resize(dim());
+        #endif
         reftmp.resize(dim());
     }
 

Diff for: lib/coek/coek/api/parameter_array.cpp

@@ -12,24 +12,35 @@ class ParameterArrayRepn : public ParameterAssocArrayRepn {
     size_t _size;
 
    public:
-    ParameterArrayRepn(size_t n) : shape({n}), _size(n) { cache.resize((size() + 1) * 2); }
+    ParameterArrayRepn(size_t n) : shape({n}), _size(n) 
+    {
+    #ifdef CUSTOM_INDEXVECTOR
+    cache.resize(2 * (size() + 1) * 2);
+    #endif
+    }
 
     ParameterArrayRepn(const std::vector<size_t>& _shape) : shape(_shape), _size(1)
     {
         for (auto n : shape)
             _size *= n;
-        cache.resize((size() + 1) * (dim() + 1));
+        #ifdef CUSTOM_INDEXVECTOR
+        cache.resize(2 * (size() + 1) * (dim() + 1));
+        #endif
     }
 
     ParameterArrayRepn(const std::initializer_list<size_t>& _shape) : shape(_shape), _size(1)
     {
         for (auto n : shape)
             _size *= n;
-        cache.resize((size() + 1) * (dim() + 1));
+        #ifdef CUSTOM_INDEXVECTOR
+        cache.resize(2 * (size() + 1) * (dim() + 1));
+        #endif
     }
 
     virtual ~ParameterArrayRepn() {}
 
+    Parameter index(const IndexVector& args);
+
     size_t dim() { return shape.size(); }
 
     size_t size() { return _size; }
@@ -72,7 +83,7 @@ void ParameterArrayRepn::generate_names()
     if (name.size() == 0)
         return;
 
-    setup();
+    expand();
 
     size_t ctr = 0;
     for (auto& param : values)
@@ -101,15 +112,18 @@ ParameterArray::ParameterArray(const std::initializer_list<size_t>& shape)
     repn->resize_index_vectors(tmp, reftmp);
 }
 
-ParameterAssocArrayRepn* ParameterArray::get_repn() { return repn.get(); }
+std::shared_ptr<ParameterAssocArrayRepn> ParameterArray::get_repn() { return repn; }
 
 Parameter ParameterArray::index(const IndexVector& args)
+{ return repn->index(args); }
+
+Parameter ParameterArrayRepn::index(const IndexVector& args)
 {
-    auto _repn = repn.get();
-    auto& shape = _repn->shape;
+    //auto _repn = repn.get();
+    //auto& shape = _repn->shape;
     assert(args.size() == shape.size());
 
-    _repn->setup();
+    expand();
 
     // We know that the args[i] values are nonnegative b.c. we have asserted that while
     // processing these arguments
@@ -120,7 +134,7 @@ Parameter ParameterArray::index(const IndexVector& args)
     if (ndx > size()) {
         // TODO - Can't we do better than this check?  Do we check if each index is in the correct
         // range?
-        std::string err = "Unknown index value: " + _repn->parameter_template.name() + "[";
+        std::string err = "Unknown index value: " + parameter_template.name() + "[";
         for (size_t i = 0; i < args.size(); i++) {
             if (i > 0)
                 err += ",";
@@ -130,7 +144,7 @@ Parameter ParameterArray::index(const IndexVector& args)
         throw std::runtime_error(err);
     }
 
-    return _repn->values[ndx];
+    return values[ndx];
 }
 
 void ParameterArray::index_error(size_t i)
@@ -181,7 +195,7 @@ ParameterArray parameter(const std::initializer_list<size_t>& shape)
 
 ParameterArray& Model::add_parameter(ParameterArray& params)
 {
-    params.repn->setup();
+    params.repn->expand();
     if (repn->name_generation_policy == Model::NameGeneration::eager)
         params.generate_names();
     else if (repn->name_generation_policy == Model::NameGeneration::lazy)

Diff for: lib/coek/coek/api/parameter_array.hpp

@@ -14,7 +14,7 @@ class ParameterArrayRepn;
 class ParameterArray : public ParameterAssocArray {
    public:
     std::shared_ptr<ParameterArrayRepn> repn;
-    ParameterAssocArrayRepn* get_repn();
+    std::shared_ptr<ParameterAssocArrayRepn> get_repn();
 
     Parameter index(const IndexVector& args);
     void index_error(size_t i);

Diff for: lib/coek/coek/api/parameter_assoc_array.cpp

@@ -13,22 +13,30 @@ namespace coek {
 
 ParameterAssocArrayRepn::ParameterAssocArrayRepn() { parameter_template.name("p"); }
 
-void ParameterAssocArrayRepn::resize_index_vectors(IndexVector& tmp,
-                                                   std::vector<refarg_types>& reftmp)
+void ParameterAssocArrayRepn::resize_index_vectors(IndexVector& tmp_,
+                                                   std::vector<refarg_types>& reftmp_)
 {
-    tmp = cache.alloc(dim());
-    reftmp.resize(dim());
+    auto dim_ = dim();
+    #ifdef CUSTOM_INDEXVECTOR
+    tmp = cache.alloc(dim_);
+    tmp_ = cache.alloc(dim_);
+    #else
+    tmp.resize(dim_);
+    tmp_.resize(dim_);
+    #endif
+    reftmp.resize(dim_);
+    reftmp_.resize(dim_);
 }
 
-void ParameterAssocArrayRepn::setup()
+void ParameterAssocArrayRepn::expand()
 {
-    if (first_setup) {
+    if (first_expand) {
         auto value = std::make_shared<ConstantTerm>(
             parameter_template.value_expression().expand().value());
         for (size_t i = 0; i < size(); i++) {
             values.emplace_back(CREATE_POINTER(ParameterTerm, value));
         }
-        first_setup = false;
+        first_expand = false;
     }
 }
 
@@ -80,11 +88,12 @@ std::vector<Parameter>::iterator ParameterAssocArray::end() { return get_repn()-
 
 #ifdef COEK_WITH_COMPACT_MODEL
 expr_pointer_t create_paramref(const std::vector<refarg_types>& indices, const std::string& name,
-                               void* var);
+                               std::shared_ptr<ParameterAssocArrayRepn>& var);
 
 Expression ParameterAssocArray::create_paramref(const std::vector<refarg_types>& args)
 {
-    return coek::create_paramref(args, get_repn()->parameter_template.name(), this);
+    auto repn = get_repn();
+    return coek::create_paramref(args, repn->parameter_template.name(), repn);
 }
 #endif
 
@@ -95,11 +104,13 @@ Expression ParameterAssocArray::create_paramref(const std::vector<refarg_types>&
 #ifdef COEK_WITH_COMPACT_MODEL
 expr_pointer_t get_concrete_param(ParameterRefTerm& paramref)
 {
-    ParameterAssocArray* param = static_cast<ParameterAssocArray*>(paramref.param);
+    //* param = static_cast<ParameterAssocArray*>(paramref.param);
+    auto param = std::dynamic_pointer_cast<ParameterAssocArrayRepn>(paramref.param);
 
     std::vector<int> index;
-    for (auto it = paramref.indices.begin(); it != paramref.indices.end(); ++it) {
-        refarg_types& reftmp = *it;
+    //for (auto it = paramref.indices.begin(); it != paramref.indices.end(); ++it) {
+    //    refarg_types& reftmp = *it;
+    for (auto& reftmp : paramref.indices) {
         if (auto ival = std::get_if<int>(&reftmp))
             index.push_back(*ival);
         else {

Diff for: lib/coek/coek/api/parameter_assoc_array.hpp

@@ -6,7 +6,8 @@ namespace coek {
 
 class ParameterAssocArray {
    public:
-    virtual ParameterAssocArrayRepn* get_repn() = 0;
+
+    virtual std::shared_ptr<ParameterAssocArrayRepn> get_repn() = 0;
     IndexVector tmp;
     std::vector<refarg_types> reftmp;
 
@@ -17,7 +18,7 @@ class ParameterAssocArray {
     size_t size();
     size_t dim();
 
-    virtual Parameter index(const IndexVector& args) = 0;
+    //virtual Parameter index(const IndexVector& args) = 0;
 #ifdef COEK_WITH_COMPACT_MODEL
     Expression create_paramref(const std::vector<refarg_types>& indices);
 #endif

Diff for: lib/coek/coek/api/parameter_assoc_array_repn.hpp

@@ -17,16 +17,21 @@ typedef std::variant<int, expr_pointer_t> refarg_types;
 
 class ParameterAssocArrayRepn {
    public:
+    #ifdef CUSTOM_INDEXVECTOR
     IndexVectorCache cache;
+    #endif
     std::vector<Parameter> values;
     Parameter parameter_template;
 
-    bool first_setup = true;
+    IndexVector tmp;
+    std::vector<refarg_types> reftmp;
+
+    bool first_expand = true;
 
    public:
     ParameterAssocArrayRepn();
 
-    virtual void setup();
+    virtual void expand();
     virtual void generate_names() = 0;
 
     virtual size_t dim() = 0;
@@ -41,6 +46,12 @@ class ParameterAssocArrayRepn {
 
     /** Set the name of the variable. */
     void name(const std::string& name);
+
+    virtual Parameter index(const IndexVector& args) = 0;
+
+#ifdef COEK_WITH_COMPACT_MODEL
+    Expression create_paramref(const std::vector<refarg_types>& indices);
+#endif
 };
 
 }  // namespace coek