[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
Benoît Jacob wrote:
"or \em dynamically by specifying \em Dynamic as the size."
Here "size" is not precise enough, I'd say "as size template parameters".
Does that really give more information?
\param _Scalar Numeric type, i.e. float, double, int
Here I prefered the old version which said:
\param _Scalar the scalar type, i.e. the type of the coefficients
Too technical. I've been reading papers on linear algebra applications
for the last couple of weeks straight and that was unclear to me.
This is kind of a common theme in my changes -- and perhaps a valid
question: Who is the target audience of the docs? Mathematicians doing
some C++ or C++ programmers doing some math? My changes are kind of
optimized for the latter -- i.e. for people that have some basic
familiarity with linear algebra, but aren't spending most of their time
in that area.
however I understand that it's a good idea to give an example 'float' but if
you give the three types as above and says "i.e." it suggests these are the
only types allowed. So I'd go for
\param _Scalar the scalar type, i.e. the type of the coefficients, for example
* \param _Rows Numer of rows, or \b Dynamic
* \param _Cols Number of columnss, or \b Dynamic
Heh, I suppose that being fixed is non-controversial. I'll fix. :-)
Moreover, why did you change this? The old version said basically the same,
with more words. Did you feel it was important to have very short
descriptions here and the old version was too wordy?
Yes, exactly. People scan API docs -- they don't read them like a book,
so it's important for them to be concise. I completely missed that bit
because there were too many words there. In my opinion, explanatory
notes should be put down below.
* \param _Rows the number of rows at compile-time. Use the special value \a
Dynamic to specify that the number of rows is dynamic, i.e. is not fixed at
* \param _Cols the number of columns at compile-time. Use the special value \a
Dynamic to specify that the number of columns is dynamic, i.e. is not fixed
Wasn't this more helpful ?
No, because I stopped reading. :-) Now if I just glance at the docs I
see what I need to do in either situation. This is of course, all in my
opinion, but since I'm a little further back from the API (i.e. I didn't
know how it was supposed to work) that perspective might be useful.
"Dynamic matrices <em>do not</em> expand dynamically."
what do you mean?
Well, that they don't, for instance function like, i.e. std::map. It's
probably obvious for someone looking at these though a mathematician's
lens that such is not the case, but looking at it as a C++ programmer
working with a container, "dynamic" in Eigen really means "fixed, just
not at compile time".
If p is a compile-time constant then the user may as well call other
convenience functions such as l1Norm() etc; if p is not, then we are probably
in the general case. I am reluctant to write code as above as it could easily
generate binary code for all paths. If such a form were really needed i'd
rather make p a template parameter and use a meta-selector so only one path
I've seen it done as a template parameter in at least one other
framework. Since they're templates, and hence being inlined, a compiler
should optimize out the if block in the case of a constant being used.
I feel like having them rolled into one function would make the API
easier to work with (less functions to scan), but of course the final
decision is with you guys.