Boost logo

Boost :

Subject: Re: [boost] [odeint] Documentation comments
From: Karsten Ahnert (karsten.ahnert_at_[hidden])
Date: 2012-09-22 13:41:54

Hi John,

thank you for reading through the docs. Here are some comments. Of
course most of your comments on spelling, grammar and formatting are
right and we will change the docs.

> *
> - "They also occur if partial differential equations (PDEs) are
> discretized in one coordinate."; wouldn't you have to discretize in all
> but one coordinate?

You are right, we will change this.

> *
> - I see no mention of any headers except the global one
> boost/numeric/odeint.hpp. Is it useful or possible to include only
> parts of the library?

It is possible to use only parts of the library. We will put a paragraph
explaining how to use only parts into this section.

> *
> - "a simple function calculating f(x)'"; I think you mean either "f(x)"
> or "x'", not "f(x)'".

Right, we will change it.
> *
> - There's one case here where you use "--" as an em dash. "on the
> example of explicit_error_rk54_ck -- a 5th order Runge-Kutta method".
> There are various other places throughout the docs which use just "-"
> for em dash. It would be best if you could use a proper em dash
> character, but in any case you should be consistent.

ok, we wil change this. It comes from LaTeX notation which we usually
use for writting documents.

> *
> - "the first numerical experiment which has been implemented on a
> computer in 1953. It was studied at Los Alamos on one of the first
> computer (a MANIAC I) and it triggered a whole new tree of mathematical
> and physical science."; This is ambiguous (it could mean the first
> experiment amongst those in 1953). If you really mean it was the first
> computational numerics experiment then you should probably say something
> like "the first numerical experiment to be implemented on a computer. It
> was studied at Los Alamos in 1953 on one of the first computers (a
> MANIAC I) and triggered a whole new tree of mathematical and physical
> science."

Yu are right, we wil change it.

> - "For example the unit test for Boost.Units take up to 4 GB of memory
> at compilation."; are you referring to the unit tests of Boost.Units, or
> a unit test of odeint that uses Boost.Units? Please clarify.

The test with odeint and Boost.Units are meant. We will clarify.

> - The matrix-as-state example has a bug:
> for( size_t j=0 ; j<x.size2() ; ++j ) dxdt( 0 , j ) = dxdt( 0 ,
> x.size1() -1 ) = 0.0;
> should be
> for( size_t j=0 ; j<x.size2() ; ++j ) dxdt( 0 , j ) = dxdt( x.size1()
> -1, j ) = 0.0;

I think you are right. I will check this.

> This bug makes me think that you ought to provide a debug mode where
> every entry in dxdt is set to NaN before calling the system operator(),
> and then they are checked to be non-NaN afterwards. Of course, this
> would only work for types with a NaN value, but I think all your
> examples do have that (except maybe the multiprecision types).

Yes, that might be possible. It sounds like a good idea. But it might
not work if someone uses a multiprecision type, or if your state type is
vor example a vector of complex numbers.

> - "for full support the min( x , y ), max( x , y ), pow( y , y ) must be
> callable"; what are x and y? Is it significant that pow has both
> arguments equal but min and max don't? I hope the requirements on the
> numeric type are laid out more formally elsewhere; please link to that.

ok, we will clarify this. And it should be pow( x , y ).

> - In the section on controlled steppers, the formulae are getting
> sufficiently complex that it might be worth trying to use TeX-generated
> pngs rather than straight HTML (as is done in Boost.Math, e.g.
> <>)

ok, we will check this.

> - I can't figure out what the three lines after table 1.6 are about.
> - The long line "boost::numeric::odeint::result_of::make_dense_output<
> r..." in Dense output steppers is too long for my screen; can you break it?
> - The resizing policy names are a bit clumsy; I think always_resize,
> initially_resize, and never_resize might be better.

you are right. I am also not happy with their names. But renaming them
might break some code. Of course, one can also introduce some typedef
for backward compatibility.

> - "Typical use cases for this kind of resizer are self expanding
> lattices like shown in the tutorial"; can you link to the relevant
> example in the tutorial?

Sure, we will do.

> *
> - Personally I think the ranges are more useful than the iterators, so
> it would be nice if they were mentioned in the section title (e.g.
> "Iterators and Ranges")

You are right. Iterators and ranges sound also good for me.

> *
> - The note "odeint already includes all the code presented here, see
> gsl_wrapper.hpp, so gsl_vectors can be used straight out-of-box. The
> following description is just for educational purpose." is duplicated in
> the subsequent paragraph.
> - Using x/y for element-wise division in vector_space_algebra seems
> potentially contentious. Similarly for abs. Perhaps it would be better
> to provide a customization point in boost::numeric::odeint as you have
> done with vector_space_reduce, whose default implementations use / and
> abs as in the current scheme. That way people will not be required to
> implement potentially misleading operators for their types.

Ok, I have to check this. I will come back to you if this is possible or

> *
> - "the state_type of the stepper must not necessarily be used to call
> the do_step method"; I'm not sure what this means; should it be "the
> state_type of the stepper need not necessarily be used to call the
> do_step method"

Yes, this meant. We will try to clarify this.

> *
> - In various places you have ScaleSum/N/ with literal '/' characters,
> where I presume they were intended to be formatting to make 'N' italics.
> - Rather than having a dedicated array_algebra, could you not have
> range_algebra automatically switch to the more efficient implementation
> when it detects a suitable State type?

Yes, in principle this is possible. You can extend the idea to introduce
a algebra and operations dispatcher which finwd the appropirate algebra
for every state type not only for ranges or arrays, but also for
Thrust's vector types or Boost.Fusion sequences for example. If people
really like to have such a feature we can implement this.

Boost list run by bdawes at, gregod at, cpdaniel at, john at