[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
Today I reworked partially the documentation of class Matrix. I tried to
preserve the spirit of the improvements that you made, by keeping the
beginning of the doc as light as possible.
However I felt the need to clarify a couple of things, I think that the API
docs must be a detailed reference documentation and not just a quick overview
(for that we have the tutorial).
Hope it's OK, feel free to comment, and SVN remembers your version in case you
don't like my changes :)
On Sunday 12 October 2008 16:06:25 Scott Wheeler wrote:
> Benoît Jacob wrote:
> > Indeed, there is no default value. I'll update the docs to mention that
> > you can use the special value Eigen::Dynamic.
> Already did. In bold. :-)
> > Yes but I said a _fixed_ size of that order of magnitude. With a size of
> > 3000000 x 24 one needs to use a dynamic-size matrix i.e. something like
> > MatrixXd m(3000000,24)
> > where MatrixXd is a typedef for Matrix<double,Dynamic,Dynamic>.
> > I'll update the docs to make clear that fixed-size matrices are allocated
> > as fixed-size arrays hence are only for small sizes.
> Already did. With \warning :-)
> > We can always add that as a convenience function, but it can't replace
> > the others. Indeed, such a function could be implemented with the formula
> > that I gave in the previous e-mail, but for the most important special
> > cases p=1,2,infinity, it would be much slower than a specialized
> > function.
> if(p == 2)
> // do it the fast way
> // do it the general way
> > I still believe that norm() should return the l2 norm, but I agree with
> > the need to clarify things much more in the documentation, I could agree
> > with adding convenience functions l1Norm() l2Norm() lInfNorm() and why
> > not lpNorm(RealScalar p) and I agree with the API change you propose
> > below:
> To be clear -- I agree that l2 is the most important norm and it makes
> sense for it to be the default for norm() -- just when norm() and
> norm2() were next to each other, it was confusing.
> Also already updated in the docs. :-)