
Boost : 
Subject: [boost] [odeint] Documentation comments
From: John Bytheway (jbytheway+boost_at_[hidden])
Date: 20120922 12:45:30
This is not a review, just a few things I've caught while reading the
odeint docs. Mostly cosmetic issues, but a couple of deeper questions.
*
http://headmyshoulder.github.com/odeintv2/doc/boost_numeric_odeint/getting_started/overview.html
 "They also occur if partial differential equations (PDEs) are
discretized in one coordinate."; wouldn't you have to discretize in all
but one coordinate?
 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?
 "a simple function calculating f(x)'"; I think you mean either "f(x)"
or "x'", not "f(x)'".
 There's one case here where you use "" as an em dash. "on the
example of explicit_error_rk54_ck  a 5th order RungeKutta 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.
*
http://headmyshoulder.github.com/odeintv2/doc/boost_numeric_odeint/tutorial/solar_system.html
 "which lead to the whole new subject of Chaos Theory"; s/lead/led/
*
http://headmyshoulder.github.com/odeintv2/doc/boost_numeric_odeint/tutorial/stiff_systems.html
 "the so called Rosenbrock method"; is "so called" necessary?
*
http://headmyshoulder.github.com/odeintv2/doc/boost_numeric_odeint/tutorial/special_topics.html
 "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."
 "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 matrixasstate 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;
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 nonNaN 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).
 "This is in principle all"; awkward phrasing, perhaps "In principle
this is all"
 I assume you know about the unfinished chunk of docs re PDEs and
equations on networks.
 "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.
 "We exemplary show this for a Hamiltonian system"; "exemplary" doesn't
work here. You could use s/exemplary show/demonstrate/.
 "simply to small"; s/to/too/
*
http://headmyshoulder.github.com/odeintv2/doc/boost_numeric_odeint/odeint_in_detail/steppers.html
 "Implicit solvers do only work"; s/do //
 "They safe a part of their history" s/safe/save/
 In the section on controlled steppers, the formulae are getting
sufficiently complex that it might be worth trying to use TeXgenerated
pngs rather than straight HTML (as is done in Boost.Math, e.g.
<http://www.boost.org/doc/libs/1_51_0/libs/math/doc/sf_and_dist/html/math_toolkit/dist/stat_tut/weg/st_eg/tut_mean_intervals.html>)
 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.
 "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?
 "So it might be suitable that the system function itself is a pair of
functions, one for computing the deterministic and one for the
stochastic part. The first element of the pair simply computes the
deterministic part while the second the stochastic one."; this is rather
redundant; how about "So it might be suitable that the system function
is a pair of functions. The first element of the pair computes the
deterministic part and the second the stochastic one."
*
http://headmyshoulder.github.com/odeintv2/doc/boost_numeric_odeint/odeint_in_detail/iterators.html
 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")
 "that is you write the above example in a shortified form";
"shortified" is not a word; perhaps "so you can write this example in a
more succinct form".
 The note "odeint already includes all the code presented here, see
gsl_wrapper.hpp, so gsl_vectors can be used straight outofbox. The
following description is just for educational purpose." is duplicated in
the subsequent paragraph.
 Using x/y for elementwise 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.
 "vector_space_reduce_impl< state_type >::reduce( state , operator ,
init )"; you probably shouldn't use the keyword "operator" as an
identifier here.
 "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"
*
http://headmyshoulder.github.com/odeintv2/doc/boost_numeric_odeint/concepts/symplectic_system.html
 Concepts are normally written without underscores, so SymplecticSystem
rather than Symplectic_System.
 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?
I hope to get a chance to play with the library later and submit a
proper review. For now, congratulations on an impressive library.
John Bytheway
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk