Joseph L Mundy wrote:
> I am soliciting opinions on the resolution of these inconsistencies:
>
> 1) One approach is to add parsing to the >> operator to match the extra
> class descriptor strings output by >>.
>
> The advantage of this approach is that useful debugging information
> would be maintained.
>
> 2) Drop the class information from <<.
>
> The stream operations will now be consistent but users will have to add
> their own class info if they need it.
>
> 3) Uniformly implement read and write methods that don’t have class info
> and reserve “print” to be more verbose.
I like 2. I think uniformly implementing operators >> and << is useful.
Putting the class name in the output is not very useful for me. It is
the client code that should decide what name to give a vgl_vector_2d.
I personally prefer operator <<() and operator >>() over member methods
like print or read. They can be implemented as either a standalone
function or member method. read, write and print force you to make a choice.
We need to be careful not to rely on the existence of any of these
methods. From our experience in writing the binary IO code in vsl, etc.
you need to be very careful in expecting any range of classes to
implement a particular set of functions.
The basic problem was providing IO for composite objects like
vcl_vector, etc. somebody will write a
template <class T> void print(const vcl_vector<T> &v, ostream &os)
{
for (unsigned i = 0; i<v.size(); ++i);
i.print(os);
}
But for those people who didn't need to print their objects, but who do
need vectors of them, this print function forces them to implement a
print method in their class. In vsl we solved this by making all binary
IO optional. In the vgl case, we could just be careful to not allow
people to put functions like the above, anywhere in the core code.
One (very disruptive, not entirely serious, but something to think
about) suggestion, is to follow the serialisation library design in
boost. There they have separated the serialisation format, from the IO
code itself. To massively simplify, you write read and write functions
void boost_serialize::write(vgl_vector_2d<T> & v, const vcl_string
&key, boost_serialise::stream &s)
{
boost_serialise::write_start_object(key, s);
boost_serialise::write(v.x(), "x", s);
boost_serialise::write(v.y(), "y", s);
boost_serialise::write_stop_object(s);
}
Then, by choosing different concrete types for the
boost_serialise::stream object, you can do binary IO similar to vsl,
(which will ignore the key, start, stop, adn "x" and "y" values.
You can do XML which will do nice indenting, value labelling, etc.
You can do highly verbose human editable text with nice indentation.
You can do very compressed text, but still somewhat edittable.
You only need to write one pair of IO function for each class, and use
it for all the different stream types. The existing vsl code could be
adapted to perform this role.
Ian.

There are some inconsistencies in the definition of stream operations in
vgl, as well as inconsistent implementation of "read" and "write"
methods. The << and >> stream operators are not consistent in that <<
outputs a class name with brackets while, the >> operator only expects
the bare parameters of the class. Also, for instance, vgl_box_3d has
read and write methods, which read and write bare parameters, while
vgl_conic_2d does not.
I am soliciting opinions on the resolution of these inconsistencies:
1) One approach is to add parsing to the >> operator to match the extra
class descriptor strings output by >>.
The advantage of this approach is that useful debugging information
would be maintained.
2) Drop the class information from <<.
The stream operations will now be consistent but users will have to add
their own class info if they need it.
3) Uniformly implement read and write methods that don't have class info
and reserve "print" to be more verbose.
Thanks in advance for your suggestions.
Joe
o----o----o----o----o----o----o----o
Professor of Engineering(Research)
Room 351
Barus and Holley
184 Hope Street
Providence, RI
401-863-2655

> Whilst this may be a relatively common idiom, it isn't strictly
> necessary - vil only uses unsigned ints as pixel indices, so checking
> for non-negativity isn't much help. You should be using unsigned ints
> for these indices in your own code. All that is left is checking for
> being outside the upper limits. Doing this explicitly doesn't take
> much more space than calling the method in_range().
This paragraph should be put in a FAQ, and/or in the vxl book, IMHO.
-- Peter.

Nicolas Burrus wrote:
> Hi,
>
> While using vil, I've run into the following flaws :
>
> 1) vil_image_view::fill(T value) is suboptimal.
> Using vcl_memset when the memory is contiguous can signicantly improve the
> performances of this function. It can be applied very easily when
> sizeof(T)==1. The attached vil_fill.patch file tries to select the best
> implementation when the image_view is contiguous. It requires template struct
> within template struct support (to avoid partial specialization), I don't
> know is this is problematic for some compilers vxl supports.
I'm not sure that this is as big a problem as it seems. Last time I
checked vil_image_view<T>::fill(T val) was optimised by my compiler to a
memset for those types which can be (vxl_byte, etc.)
I could be wrong here, and if you have any evidence to the contrary, I'd
be pleased to change the code.
I do think that a check for is_continuous could usefully be put in
vil_fill.h.
>
> 2) I could not find any equivalent to vil1_image::in_range with vil, so I
> added an in_range member to vil_image_view. This is in_range.patch.
Whilst this may be a relatively common idiom, it isn't strictly
necessary - vil only uses unsigned ints as pixel indices, so checking
for non-negativity isn't much help. You should be using unsigned ints
for these indices in your own code. All that is left is checking for
being outside the upper limits. Doing this explicitly doesn't take much
more space than calling the method in_range().
Given that the design plan for vil calls for the use of minimal
interfaces for each class, I'd rather not put in_range() into
vil_image_view<T>.
>
> 3) I could not find any vil_math_image_max/min function, the compute the max
> or the min between two images. Neither I could find a generic apply function,
> taking 2 images, a binary functor, and an output image. I think I will write
> one if nobody's is against it. For the moment, vil_image_math_maxmin.patch
> adds vil_math_image_max and min functions.
I'll add these. Thank-you very much for the tests.
BTW: It isn't quite as efficient, but if you have other similar
functions, they can be easily implemented in terms of vil_transform().
> I'm new to vxl and its philosophy, please let me know if I'm not respecting
> the spirit nor the constraints of vxl.
Your suggestions are very welcome. Your code was very well written,
tested, and presented. The fact that you sent us patches rather than the
usual "Please can you fix X to do Y" is exemplarary.
As a general rule, suggestions to change core classes (vil_image_view,
vnl_vector, etc.) are held to pretty high standard. Other code vin
VXLSRC/core to a less high standard (adherence to code-style and
documentation guidelines is usually enough). Code in contrib, can be
however the authors choose.
Efficiency improvements are most welcome when sent with timing evidence.
It is especially useful if the timing test is left in the code, so that
it can be tested on other platforms. (e.g. see
core/vil/tests/vil_test_random_access_timings.cxx)
Thanks for your contributions.
Ian.