[vtk-developers] comments inside BTX ETX + matrix variable naming

Marcus D. Hanwell marcus.hanwell at kitware.com
Sat Dec 12 14:55:51 EST 2009

On Sat, Dec 12, 2009 at 2:04 PM, David Doria
<daviddoria+vtk at gmail.com<daviddoria%2Bvtk at gmail.com>
> wrote:

> On Sat, Dec 12, 2009 at 10:32 AM, Marcus D. Hanwell
> <marcus.hanwell at kitware.com> wrote:
> > On Sat, Dec 12, 2009 at 10:23 AM, David Doria <daviddoria+vtk at gmail.com<daviddoria%2Bvtk at gmail.com>
> >
> > wrote:
> >>
> >> On Sat, Dec 12, 2009 at 10:17 AM, Francois Bertel
> >> <francois.bertel at kitware.com> wrote:
> >> > Arguments of functions and methods in VTK start with lowercase. So
> >> > it's is not OK. Only the variable member of a class start with an
> >> > upper case.
> >>
> >> It seems very annoying to not comply with mathematics notation, which
> >> is clearly more standard than VTK notation.
> >>
> > The mathematical notation is there to make it clear that the variable in
> the
> > formula is a matrix, this is C++ code in a matrix class, which already
> makes
> > it clear that these variables are concerned with matrix algebra.
> > It is fine in the comments to use conventional mathematical notation, but
> I
> > agree with Francois that the code should stick to VTK coding conventions
> > wherever possible. Having comments inside of a //BTX //ETX block does not
> > present any issues.
> > I think the comments are a little on the verbose side, I would rather go
> > into detail about how the matrix is laid out in memory in the class
> > description, and then the individual function documentation can be much
> > briefer. So explain row major order, assume the reader knows about matrix
> > algebra in the class description and then keep brief function
> descriptions.
> > How does that sound? I am hoping to add some more inline doxygen code
> > examples to some of the new code I have added, examples of which can
> already
> > be seen (not by me) in the OpenGLExtensionManager header.
> > Marcus
> Marcus,
> Thanks for the thoughtful response. I have a few responses/comments below:
> 1)
> The comments can't use capital letters for matrices if the prototypes
> do not follow suit. For example, this doesn't make sense:
>  // Multiplies matrices A and B and stores the result in C according to
>  // C = AB.
>  static void Multiply3x3(vtkMatrix3x3 *a, vtkMatrix3x3 *b, vtkMatrix3x3 *c)
> Because A,B,C are undefined to the user as a,b,c are the only things
> that appear in the function (clearly it is trivial in this case, but
> we are trying to create guidelines here).
> My point once more is that this:
>  // Multiplies matrices a and b and stores the result in c according to
>  // c = ab.
>   static void Multiply3x3(vtkMatrix3x3 *a, vtkMatrix3x3 *b, vtkMatrix3x3
> *c)
> doesn't "look like" a matrix multiplication, it "looks like" more of
> an inner product of vectors or something, know what I mean. Again, in
> this simple case it is not so important, but with other functions
> where the user would not immediately know what is going on with the
> implementation it would be helpful for the notation to match that
> which he is used to.

The point here is that coding conventions form an important cornerstone of
development when writing large libraries, and must be consistently applied.
After having worked on several different libraries and applications in the
past (one of which was a C++ template linear algebra library) I think that
coding conventions trump mathematical conventions *in the code*.

That being said you can still write something along the lines of

// Multiplies matrices A and B and stores the result in C according to
// C = AB.
// where A, B, and C correspond to the vtkMatrix3x3 arguments a, b, and c

There are several other conventions around matrix algebra such as the use of
bold type that are also not applicable in function signatures. It is a
slippery slope when you start making exceptions because of conventions in
other fields, and I would err on the side of using an extra line in the
comments to explain the correspondence of the C++ objects to the
mathematical constructs they represent.

> 2) I REALLY like the idea of little code snippets in the
> documentation. Ideally, in EVERY class I think there should be
> a) a little snippet with each non-trivial function

Not sure I would go this far, but I certainly think in the main description
and then for the functions that it makes sense for. This would have to be
added gradually over time, but in my opinion a few well placed examples can
really soften the learning curve.

> b) a link to a compilable example usage of the class (the things I've
> been putting on the wiki)

This is a good idea, although in many ways I prefer the examples in VTK as
dashboards break when they cease to compile. I wonder if the examples on the
wiki can be effectively pulled/synced with examples under version control
that get built.

> c) a one line "Necessary libraries: vtkHybrid, vtkWidgets" that tells
> the user which libraries they will need to link to in order to use
> this class.

This would certainly be useful, and we have talked about this before. I
wonder if this belongs in descriptions of the VTK kits, and whether CMake
could take care of some of these lower level details for most projects.

> Thoughts on these 3 things? If you think these snippets/verbose
> descriptions clutter the code (which they certainly would), should we
> start a "VTK Software Guide" mimicking the "ITK Software Guide"? It
> would be a big project to build and maintain, but I think it would be
> invaluable. What would HAVE to happen is some real delegation and team
> work of the whole community. I feel too often that we do not leverage
> the huge numbers of VTK users for things like this.

I think that the extra documentation in the headers is great for both
reading the doxygen and when actually editing the code. It would be good to
know how the more seasoned VTK developers feel about this. Looking at
examples such as Qt (which inspired the creation of Doxygen) they put a lot
of documentation into their code.

When the documentation can be brief it should be though. Explaining general
concepts in the full description, linking to other sources where appropriate
(i.e. no need to explain matrix algebra, at most a link to a good source or
two for background, then just this function matrix multiplies a and b,
storing the result in c).

> 3)
> For most functions, the memory storage convention (row major) doesn't
> matter - in fact, I thought this was the whole idea of providing a
> Matrix3x3 class - why would anyone use a 9-vector to represent a
> matrix? Should that even be supported? It seems like it is just asking
> for trouble and there is already a good solution in place. That said,
> if I describe this in the class documentation, can we then say for all
> the function that take a double[9] something like "See the class
> description for a description of the memory storage convention."?

They are useful for efficiency, especially when heavy vtkObjects need to be
used. So I think they should be supported, but for code where performance is
not critical the C++ objects are preferred. If we supported lighter C++
stack based objects, then I would be in favor of removing the naked C arrays

> The WORST case scenario is for the user to see the function and say
> "Man this is annoying, I wonder which order the elements are supposed
> to be in? I'm going to have to make a little demo and try it to make
> sure I know what it is doing before I use it in my code". What it is
> doing MUST be explained. The "little demo" MUST be provided. Moving
> the responsibility of process from the user to the toolkit is pretty
> much my whole motivation for this "documentation clean up" effort.

True, although C++ convention dictates part of this. Stating the storage
order certainly can't hurt though. I think that these are useful discussions
to have, and I would like to improve the VTK docs. I think it will be a
gradual process, but a welcome one for many new VTK users and developers

Marcus D. Hanwell, Ph.D.
R&D Engineer, Kitware Inc.
(518) 881-4937
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20091212/275b6691/attachment.html>

More information about the vtk-developers mailing list