Re: [eigen] maxCoeff taking reference?

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]


trying to understand the justification here. wouldn't pointers as out parameters easily lead to bugs, such as null pointers, or wild pointers? as I read today's [eigen source here](https://github.com/eigenteam/eigen-git-mirror/blob/667b550a1f161ac85bc8f2013939a0adc10af12c/Eigen/src/Core/Visitor.h#L278), `*rowPtr = maxVisitor.row;` looks like a real bug. with references you
don't get this kind of bugs, so why not?

as for maxCoeff to return coord, does it make sense to use tags to express programmer intention, e.g.,
auto [value, row, col] = A.maxCoeff(Eigen::return_coord);

basically a form of tag dispatching, but only for overload purpose to live with the original version to cover all usage.

On Fri, Mar 22, 2019 at 9:20 AM David Tellenbach <david.tellenbach@xxxxxxxxxxxxx> wrote:
Hello,

> We must make sure that this does not introduce overhead if the index(es) is not used. Perhaps with some template magic it is possible to distinguish calling
>
>   auto [value, row,col] = A.maxCoeff();
> vs
>   auto value = A.maxCoeff();

I guess it is not possible to realise something like this without (unnecessarily) calculating the indices in the case

    auto value = A.maxCoeff();

If we allow the unnecessary index calculation this is easy to implement. Distinguishing function calls based on return types is usually really hard because return types are not used for template deduction.

>  If that is not possible, we could of course just give one method a different name.

Another alternative would be a static function maxCoeff(A) that returns a struct (or a tuple) but this might introduce more confusion.

Cheers,
David

> On 22. Mar 2019, at 11:54, Christoph Hertzberg <chtz@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> We must make sure that this does not introduce overhead if the index(es) is not used. Perhaps with some template magic it is possible to distinguish calling
>
>   auto [value, row,col] = A.maxCoeff();
> vs
>   auto value = A.maxCoeff();
>
> This would actually be very close to how Matlab "overloads" many functions. If that is not possible, we could of course just give one method a different name.
>
> Btw: I agree on not overloading for references, as it should be obvious that these are out-parameters. O.t.o.h., with pointers there is always the ambiguity if null-pointers are allowed or not.
>
> Christoph
>
> On 22/03/2019 09.45, Gael Guennebaud wrote:
>> We chose pointers to emphasise they are out parameters. I would not add
>> overloads taking references but maybe a version returning everything within
>> a struct to be used with C++17 structure bindings?
>> On Thu, Mar 21, 2019 at 3:27 AM Meng Zhu <meng@xxxxxxxxxxxxx> wrote:
>>> hi, I noticed eigen maxCoeff function (
>>> https://eigen.tuxfamily.org/dox/classEigen_1_1DenseBase.html#a784e23ccbb39e7c57b70af386f94f8b5)
>>> takes pointers to return the max entry coord, and there is no overload to
>>> take reference type. is there a reason to not provide reference access?
>>> thanks.
>>>
>>> Meng
>>>
>>>
>>>
>
> --
> Dr.-Ing. Christoph Hertzberg
>
> Besuchsadresse der Nebengeschäftsstelle:
> DFKI GmbH
> Robotics Innovation Center
> Robert-Hooke-Straße 5
> 28359 Bremen, Germany
>
> Postadresse der Hauptgeschäftsstelle Standort Bremen:
> DFKI GmbH
> Robotics Innovation Center
> Robert-Hooke-Straße 1
> 28359 Bremen, Germany
>
> Tel.:     +49 421 178 45-4021
> Zentrale: +49 421 178 45-0
> E-Mail:   christoph.hertzberg@xxxxxxx
>
> Weitere Informationen: http://www.dfki.de/robotik
>  -------------------------------------------------------------
>  Deutsches Forschungszentrum für Künstliche Intelligenz GmbH
>  Trippstadter Strasse 122, D-67663 Kaiserslautern, Germany
>
>  Geschäftsführung:
>  Prof. Dr. Jana Koehler (Vorsitzende)
>  Dr. Walter Olthoff
>
>  Vorsitzender des Aufsichtsrats:
>  Prof. Dr. h.c. Hans A. Aukes
>  Amtsgericht Kaiserslautern, HRB 2313
>  -------------------------------------------------------------
>
>
>





Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/