[ Thread Index |
Date Index
| More lists.tuxfamily.org/eigen Archives
]
Dear All,
Am 12.09.2016 um 16:53 schrieb Christoph Hertzberg:
On 2016-09-12 16:37, Pavel Holoborodko wrote:
I'd rather stick with std::numeric_limits::min():
I second that.
I'm fine with either way.
Of course we should then update our documentation on how to provide custom types (i.e., explicitly say that one should specialize std::numeric_limits and inherit NumTraits<...> from GenericNumTraits<...>).
Probably the historic reason for introducing the aliases into NumTraits was to avoid the confusion with min() for integers vs min() for floating point types)
A small advantage of having aliases in NumTraits would be that we could
typedef NumTraits<Scalar> NT;
locally, in case we need multiple attributes of it.
o.k., I trust you if you say it's fine to overload numerics_limits.
I changed my test code using
namespace std
{
template<> class numeric_limits<TpRFloat>
{
public:
static TpRFloat min() { return TpRFloat( numeric_limits<TpFloatBase>::min(), numeric_limits<TpFloatBase>::min() ); };
};
}
and if works fine.
Documenting it is probably a good idea, as forgetting to specialize numeric_limits produces wrong code for custom types without any warning.
BTW, will one also need the const and the reference versions, like numeric_limits<const TpRFloat> , or are they never called?
Best regards,
Peter
It is good idea to specialize std::numeric_limits for custom numeric type.
Together with std::swap, which enables std algorithms to work with the
type efficiently.
And probably provide specialization of std::complex for the type.
Some time ago I actually tried to make unit tests for everything with a minimal-example generic type (i.e., just encapsulating a float and forwarding all operators and std-functions). One of the main obstacles was that we often do
things like
Scalar(1) or Scalar(0)
which does not work, if Scalar=std::complex<CustomType> and CustomType does not have an implicit constructor from int (and writing Scalar(RealScalar(0)) everywhere would be annoying).
How should a user best specialize std::complex to avoid that (without copying the whole std::complex implementation, and just adding another constructor)? Unfortunately, one can't do
namespace std {
template<>
class complex<MyType> : public ::std::complex<MyType>
{/* add stuff here */};
}
Could/should we maybe make our own internal Eigen::complex type (which inherits from std::complex, but adds the additional constructors)?
This would also make it easier to provide fixed abs() implementations for compilers where it is broken (I briefly remember some issues with that).
Christoph
On Mon, Sep 12, 2016 at 11:23 PM, Gael Guennebaud
<gael.guennebaud@xxxxxxxxx> wrote:
I'd rather stick with std::numeric_limits::min():
std::numeric_limits has to be specialized for custom types anyways (it is
one of the rare cases where users are welcomed to enter in the std
namespace), and I don't see reason for exposing a different value for
std::numeric_limits::min() and NumTraits::smallest()? The only advantage I
see could be a clearer name (not sure smallest is really clearer though).
I know that we already have some aliases in NumTraits, but retrospectively I
think this was a bad idea, it just make everything more confusing.
gael
On Mon, Sep 12, 2016 at 11:08 AM, Peter <list@xxxxxxxxxxxxxxxxx> wrote:
Dear Christoph,
Am 12.09.2016 um 10:57 schrieb Christoph Hertzberg:
If nobody objects against that, I'll patch this into the dev-branch.
Especially, are there objections against or alternative suggestions for the
naming? `lowest()` probably would be a bad choice, since this is what std
calls what
we call `min()`.
The problem with std::min() is, that it has to be understood historically.
The name is a bad choice,
since the behaviour is different for integer and floating point values .
N.B.: The 1 must be T(1) here (in case T is not implicitly constructible
from int). And perhaps we should document it as "lowest non-denormalized
positive value".
Which would suggest lowest() as a name, which is however already used.
Best regards,
Peter