Re: [eigen] docs updates

[ Thread Index | Date 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 \c float.

***

  * \param _Rows Numer of rows, or \b Dynamic
  * \param _Cols Number of columnss, or \b Dynamic

typos :)

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.

For reference:

* \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 compile-time. * \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 at compile-time.

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 gets compiled.

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.

-Scott

---


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