eudoxos / minieigen

Boost::Python wrapper for parts of the Eigen c++ library
33 stars 15 forks source link

change return type for `get_item` #15

Open ofloveandhate opened 6 years ago

ofloveandhate commented 6 years ago

the return type for VectorVisitor's get_item is as in this line of code:

static Scalar get_item(const VectorT& self, Index ix){ IDX_CHECK(ix,dyn()?(Index)self.size():(Index)Dim); return self[ix]; }

However, for numeric types which need heap allocation (multiprecision types), this causes the return of a brand-new object. But the problem is not limited to the heap allocation, which could possibly be benign. In my case, the contents of the vectors for multiprecision types are in fact of variable precision. Observe:

import pybertini
pybertini.default_precision() # gives 16
v = pybertini.multiprec.Vector(2) # make a vector of default-constructed multiprecision complex numbers (0's)
pybertini.default_precision(20) # change precision of future-made objects.  does not affect current objects.
v[0].precision() # erroneously gives 20, should give 16

So, if I make a vector of multiprecision numbers at one precision, then change the default precision to something else, and then get an element of that same vector -- which has not changed precision -- i get

  1. a new copy of that entry
  2. at the wrong precision (doesn't actually match the internal precision of the vector).

I hence propose a modification to the VectorVisitor's get_item -- that it return an internal reference. However, perhaps this is not the desired behaviour -- should I modify minieigen so that the decision of internal reference vs copy is a policy?

Pull request will be incoming..

ofloveandhate commented 1 year ago

I think this one stands as needing attention.

eudoxos commented 1 year ago

Go ahead and fork. Minieigen was developed with plain (machine) numeric types where this was never an issue. I think there must have been some reason for not returning internal references for plain numeric types. Perhaps you find a way to return internal reference for multiprecision types and keep the current behavior for plain numeric types.