From: Amitha P. <pe...@cs...> - 2003-12-17 14:55:43
|
Hi all In vil, I'd like to change all the parameter use to select the output template type from T() to (T*)0. For example, change vil_convert_cast( float(), img ); to vil_convert_cast( (float*)0, img ); Details: A function template can have template parameters that cannot be deduced from the arguments. For example, in template<class T> foo( int ); a call such as foo( 5 ); cannot deduce the type T. The Standard C++ way to resolve the ambiguity is thus: foo<int>( 5 ); // T == int foo<float>( 5 ); // T == float However, many of the current compilers cannot parse this syntax. Therefore we have a workaround template<class T> foo( T dummy, int ); // foo1 or template<class T> foo( T* dummy, int ); // foo2 The calls above are then written as foo( int(), 5 ); foo( float(), 5 ); or foo( (int*)0, 5 ); foo( (float*)0, 5 ); vil current uses option 1 (foo1). I want to move to option 2 (foo2). The reason is overloads. For example, the overloads // overload 1 template<class inT1, class outT1> void convert( vil_image_view<inT1> const& a, vil_image_view<outT1>& b ) // overload 2 template<class inT2, class outT2> vil_image_view<outT2> convert( outT2 /*dummy*/, vil_image_view<inT2>& b ) are ambiguious when called like vil_image_view<byte> im1; vil_image_view<double> im2; convert( im1, im2 ) because it can equally well match overload 1 with inT1=byte and outT1=double or match overload 2 with inT2=double and outT2=vil_image_view<byte>. Using option 2 (foo2), overload 2 becomes // overload 2' template<class inT2, class outT2> vil_image_view<outT2> convert( outT2* /*dummy*/, vil_image_view<inT2>& b ) The call then unambigiously calls overload 1 because there is no pointer to match overload 2'. This is a pervasive API change. It is difficult (extremely cumbersome) to make this change using the deprecation mechanism, so I'd like to just do it. But I can wait for another release cycle first, if necessary. Comments? Amitha. |
From: Ian S. <ian...@st...> - 2003-12-17 15:29:46
|
Short answer. I think another solution would be more appropriate. Long answer. Amitha's proposed solution raises the following issues. 1. The dummy T() mechanism is currently used throughout vil and vxl. Either vil_convert will become a special case, or the rest of vil and vxl should be converted. 2. There is risk that some things will fail silently - especially if you convert the rest of vil to use (T*)(). 3. Manchester (at least) uses some of the vil_convert code a bit, and other vil code using dummy T() a lot, and we would prefer not to change our code. OTOH, the vil_convert.h file has been one of the most modified files in vil, and I'm not yet convinced that we have it correct. I am quite happy to see vil_convert.h change again, but preferably only if a. nothing fails silently b. the API follows patterns used in the rest of the vil and vxl. c. it is easy for new users. It seems to me that the problem here is an overload resolution between void vil_convert_xxx{vil_image_view<T> in, vil_image_view<T> out) and vil_image_view<T> vil_convert_xxx{vil_image_view<T> in, T) The overload disambiguation could be more simply achieved by renaming of the latter versions of vil_convert_xxx, eg. to vil_convertfrom_xxx. This would also involve creating vil_convertfrom_xxx(vil_image_view_base_sptr &, T dummy) and deprecating the old vil_convert_xxx(vil_image_view_base_sptr &, T dummy) I think that this solution fits in more with existing vxl practice, involves less hassle to existing users, and may even be easier for new users. There are other solutions along these lines (e.g. deprecate vil_convert.h create a vil_translate.h.) that I would be happy with. Ian. > -----Original Message----- > From: Amitha Perera [mailto:pe...@cs...] > Sent: Wednesday, December 17, 2003 2:55 PM > To: vxl...@li... > Subject: [Vxl-maintainers] Selecting the type for templated functions > > > Hi all > > In vil, I'd like to change all the parameter use to select the output > template type from T() to (T*)0. For example, change > vil_convert_cast( float(), img ); > to > vil_convert_cast( (float*)0, img ); > > > Details: > > A function template can have template parameters that cannot be > deduced from the arguments. For example, in > > template<class T> foo( int ); > > a call such as > > foo( 5 ); > > cannot deduce the type T. The Standard C++ way to resolve the > ambiguity is thus: > > foo<int>( 5 ); // T == int > foo<float>( 5 ); // T == float > > However, many of the current compilers cannot parse this > syntax. Therefore we have a workaround > template<class T> foo( T dummy, int ); // foo1 > or > template<class T> foo( T* dummy, int ); // foo2 > > The calls above are then written as > > foo( int(), 5 ); > foo( float(), 5 ); > > or > > foo( (int*)0, 5 ); > foo( (float*)0, 5 ); > > vil current uses option 1 (foo1). I want to move to option 2 > (foo2). The reason is overloads. For example, the overloads > > // overload 1 > template<class inT1, class outT1> > void convert( vil_image_view<inT1> const& a, > vil_image_view<outT1>& b ) > > // overload 2 > template<class inT2, class outT2> > vil_image_view<outT2> convert( outT2 /*dummy*/, > vil_image_view<inT2>& b ) > > are ambiguious when called like > > vil_image_view<byte> im1; > vil_image_view<double> im2; > convert( im1, im2 ) > > because it can equally well match overload 1 with inT1=byte and > outT1=double or match overload 2 with inT2=double and > outT2=vil_image_view<byte>. > > Using option 2 (foo2), overload 2 becomes > > // overload 2' > template<class inT2, class outT2> > vil_image_view<outT2> convert( outT2* /*dummy*/, > vil_image_view<inT2>& b ) > > The call then unambigiously calls overload 1 because there is no > pointer to match overload 2'. > > This is a pervasive API change. It is difficult (extremely cumbersome) > to make this change using the deprecation mechanism, so I'd like to > just do it. But I can wait for another release cycle first, if > necessary. > > Comments? > > Amitha. > > > ------------------------------------------------------- > This SF.net email is sponsored by: IBM Linux Tutorials. > Become an expert in LINUX or just sharpen your skills. Sign > up for IBM's > Free Linux Tutorials. Learn everything from the bash shell > to sys admin. > Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click > _______________________________________________ > Vxl-maintainers mailing list > Vxl...@li... > https://lists.sourceforge.net/lists/listinfo/vxl-maintainers > |
From: Amitha P. <pe...@cs...> - 2003-12-17 16:27:53
|
Ian Scott wrote: > Amitha's proposed solution raises the following issues. > 1. The dummy T() mechanism is currently used throughout vil and vxl. Either > vil_convert will become a special case, or the rest of vil and vxl should be > converted. I don't see that. From a quick glance, this mechanism is used in vil and in rrel. vil uses T, rrel uses T*. There isn't much to call either way a special case. There are pros and cons to each--overload resolution, requirement for a default constructor, scriptable changes, etc. It's not worth arguing, though. Both are good enough as a work around. However, I think the T approach *with T as the first parameter* is a good standard to follow, because foo( T(), real_args ); can easily be scripted to and from the "proper" foo<T>( real_args ); > 2. There is risk that some things will fail silently - especially if you > convert the rest of vil to use (T*)(). I can't think of a case where a T will be silently converted to a T*, except for the literal 0. The literal 0 is never used alone for this purpose, so that situation will not arise. > 3. Manchester (at least) uses some of the vil_convert code a bit, and other > vil code using dummy T() a lot, and we would prefer not to change > our code. This is an important consideration, with enough weight to veto the proposed changes. > It seems to me that the problem here is an overload resolution between > void vil_convert_xxx{vil_image_view<T> in, vil_image_view<T> out) > and > vil_image_view<T> vil_convert_xxx{vil_image_view<T> in, T) As I suggest above, that should be vil_image_view<T> vil_convert_xxx( T, vil_image_view<T> in ) > The overload disambiguation could be more simply achieved by > renaming of the latter versions of vil_convert_xxx, eg. to > vil_convertfrom_xxx. I agree. It makes sense that two functions that do different things are called by different names. I suggest calling the function-style call vil_convert_xxx_as, so that the calls read nicely: dest = vil_convert_cast_as( float(), src ); // output is a float image dest = vil_convert_stretch_range_as( byte(), src ); // output is a byte image dest = vil_convert_to_grey_using_average_as( double(), src ); The last does read as nicely as the first two, but "as" reads better than "to" here. The functions where is doesn't read nicely may not need a function-style version anyway. > There are other solutions along these lines (e.g. deprecate vil_convert.h > create a vil_translate.h.) that I would be happy with. This might be a good solution to avoid overloading issues and such with deprecated functions. Does anyone have an objection to using vil_translate_cast instead of vil_convert_cast in future? Amitha. |
From: Ian S. <ian...@st...> - 2003-12-17 17:00:09
|
> From: Amitha Perera [mailto:pe...@cs...] > > Ian Scott wrote: > > Amitha's proposed solution raises the following issues. > > 1. The dummy T() mechanism is currently used throughout vil > and vxl. Either > > vil_convert will become a special case, or the rest of vil > and vxl should be > > converted. > > I don't see that. From a quick glance, this mechanism is used in vil > and in rrel. vil uses T, rrel uses T*. There isn't much to call either > way a special case. There are pros and cons to each--overload > resolution, requirement for a default constructor, scriptable changes, > etc. It's not worth arguing, though. Both are good enough as a work > around. No - you're right, it isn't widely used outside vil. My confusion. > ... > > 2. There is risk that some things will fail silently - > especially if you > > convert the rest of vil to use (T*)(). > > I can't think of a case where a T will be silently converted to a T*, > except for the literal 0. The literal 0 is never used alone for this > purpose, so that situation will not arise. I am still worried about creating other overloading ambiguities, e.g. 1. (T*) conversions to vil_image_view_base_sptrs in vil_convert 2. confusion between T* dummy_accumulation_type and S* kernel_ptr in vil_convolve, etc. > ... > I suggest calling the function-style call vil_convert_xxx_as, so that > the calls read nicely: > > dest = vil_convert_cast_as( float(), src ); // output is a > float image > dest = vil_convert_stretch_range_as( byte(), src ); // > output is a byte image > dest = vil_convert_to_grey_using_average_as( double(), src ); > > The last does read as nicely as the first two, but "as" reads better > than "to" here. I like vil_convert_xxx_as(). I suspect that this is the least impact change. > This might be a good solution to avoid overloading issues and such > with deprecated functions. Does anyone have an objection to using > vil_translate_cast instead of vil_convert_cast in future? When I want talk about int a = (float) b, I think "conversion" as does the C++ literature. I expect that moving to vil_translate.h will require more work the using vil_convert_xxx_as() solution. However, if there are reasons for not using vil_convert_xxx_as(), then ok. Ian. |
From: Tim C. <Tim...@ma...> - 2003-12-18 09:28:56
|
Amitha Perera wrote: > > >This might be a good solution to avoid overloading issues and such >with deprecated functions. Does anyone have an objection to using >vil_translate_cast instead of vil_convert_cast in future? > > > I much prefer the "vil_convert_xxx_as" approach. To me "translating" an image means moving it sideways rather than converting it to a different type. But maybe I work with geometry too much. Tim -- Tim Cootes Senior Lecturer Imaging Science and Biomedical Engineering tel (+44) 0161 275 5146 The University of Manchester fax (+44) 0161 275 5145 Manchester M13 9PT , UK mailto:t.c...@ma... http://www.isbe.man.ac.uk/~bim |
From: William A. H. <bil...@ny...> - 2003-12-17 17:03:08
|
VNL_C_VECTOR_USE_VNL_ALLOC by default is not thread safe. Is there a possibility of changing this? The problem is that ITK is multi-threaded and uses vnl in threads sometimes. We want the ITK folks to be able to link to an existing vxl installation but things will not work if the default has not been set correctly. Also, how is this set from running cmake? It looks like to get the thread safe version you have to edit the CMakeLists.txt file. -Bill |
From: Peter V. <Pet...@es...> - 2003-12-17 17:15:48
|
> VNL_C_VECTOR_USE_VNL_ALLOC by default is not thread safe. > Is there a possibility of changing this? By default, VNL_C_VECTOR_USE_VNL_ALLOC should be OFF, since VNL_CONFIG_THREAD_SAFE is set to 1 in vnl/vnl_config.h I suppose that the lines concerning VNL_C_VECTOR_USE_VNL_ALLOC in vnl/CMakeLists.txt should be removed, so that the choice for thread-safeness is decided in vnl/vnl_config.h -- Peter. |
From: Brad K. <bra...@ki...> - 2003-12-18 21:30:59
|
On Wed, 17 Dec 2003, Peter Vanroose wrote: > > VNL_C_VECTOR_USE_VNL_ALLOC by default is not thread safe. > > Is there a possibility of changing this? > > By default, VNL_C_VECTOR_USE_VNL_ALLOC should be OFF, since > VNL_CONFIG_THREAD_SAFE is set to 1 in vnl/vnl_config.h > > I suppose that the lines concerning VNL_C_VECTOR_USE_VNL_ALLOC > in vnl/CMakeLists.txt should be removed, so that the choice for > thread-safeness is decided in vnl/vnl_config.h From core/vnl/CMakeLists.txt: # VNL_ALLOC is a fast but currently thread-unsafe allocator # used in vnl_vectors. The default is to use it but to # use the slower, thread-safe allocation, in your # CMakeListsLocal.txt file add the following lines # SET(VNL_C_VECTOR_USE_VNL_ALLOC_DEFINED 1) # ADD_DEFINITIONS(-DVNL_C_VECTOR_USE_VNL_ALLOC=0) IF(NOT VNL_C_VECTOR_USE_VNL_ALLOC_DEFINED) ADD_DEFINITIONS(-DVNL_C_VECTOR_USE_VNL_ALLOC=1) ENDIF(NOT VNL_C_VECTOR_USE_VNL_ALLOC_DEFINED) From core/vnl/vnl_c_vector.txx: #ifdef VNL_C_VECTOR_USE_VNL_ALLOC // if set in build environment, go with that. #else // else, see what vnl_config.h has to say about it. # include "vnl_config.h" # if VNL_CONFIG_THREAD_SAFE # define VNL_C_VECTOR_USE_VNL_ALLOC 0 # else # define VNL_C_VECTOR_USE_VNL_ALLOC 1 # endif #endif This does in fact use the non-thread-safe version by default. It also makes switching the default result in funny behavior. No settings like this should be controlled by adding a -D to the command line of the compiler. When someone changes it, there is no dependency to go back and rebuild the .o files affected by the macro. I suggest we change the setting to be an option configured into a header file with a default of off. Perhaps vnl_config.h should be a configured header with such options. Thoughts? -Brad |
From: Peter V. <Pet...@es...> - 2003-12-19 07:47:44
|
> I suggest we change the setting to be an option configured into a header > file with a default of off. Perhaps vnl_config.h should be a configured > header with such options. Agreed. I propose to remove that section from CMakeLists.txt, so that the settings in vnl_config.h take effect: @@ -153,15 +153,12 @@ ENDIF(CMAKE_COMPILER_IS_GNUCXX) # VNL_ALLOC is a fast but currently thread-unsafe allocator -# used in vnl_vectors. The default is to use it but to -# use the slower, thread-safe allocation, in your -# CMakeListsLocal.txt file add the following lines +# used in vnl_vectors. The default is to +# use the slower, thread-safe allocation. +# To change this, either add the following lines to your +# CMakeListsLocal.txt file, or modify vnl_config.h as explained there. # SET(VNL_C_VECTOR_USE_VNL_ALLOC_DEFINED 1) # ADD_DEFINITIONS(-DVNL_C_VECTOR_USE_VNL_ALLOC=0) - -IF(NOT VNL_C_VECTOR_USE_VNL_ALLOC_DEFINED) - ADD_DEFINITIONS(-DVNL_C_VECTOR_USE_VNL_ALLOC=1) -ENDIF(NOT VNL_C_VECTOR_USE_VNL_ALLOC_DEFINED) ADD_LIBRARY(vnl ${vnl_sources}) TARGET_LINK_LIBRARIES( vnl vcl ) Actually, I never noticed the problem in my non-CMake builds ;-) -- Peter. |
From: Brad K. <bra...@ki...> - 2003-12-19 14:01:56
|
On Fri, 19 Dec 2003, Peter Vanroose wrote: > > I suggest we change the setting to be an option configured into a header > > file with a default of off. Perhaps vnl_config.h should be a configured > > header with such options. > > Agreed. I propose to remove that section from CMakeLists.txt, so that the > settings in vnl_config.h take effect: I suggest that we make vnl_config.h a configured header: // vxl/core/vnl/vnl_config.h.in #define VNL_CONFIG_CHECK_BOUNDS @VNL_CONFIG_CHECK_BOUNDS@ #define VNL_CONFIG_LEGACY_METHODS @VNL_CONFIG_LEGACY_METHODS@ #define VNL_CONFIG_THREAD_SAFE @VNL_CONFIG_THREAD_SAFE@ Then these options would be available as advanced cache entries in CMake or as --vnl-config-foo options in configure. The header would be configured to the build tree like this: // vxl-build/core/vnl/vnl_config.h #define VNL_CONFIG_CHECK_BOUNDS 1 #define VNL_CONFIG_LEGACY_METHODS 0 #define VNL_CONFIG_THREAD_SAFE 1 No other code would have to change. When one of these options changes, all dependent source files are automatically recompiled. This would also allow us to remove the "edit but do not commit" comments from the file. Comments? -Brad |
From: Amitha P. <pe...@cs...> - 2003-12-19 14:28:09
|
Brad King wrote: > I suggest that we make vnl_config.h a configured header: I agree. > // vxl-build/core/vnl/vnl_config.h > #define VNL_CONFIG_CHECK_BOUNDS 1 > #define VNL_CONFIG_LEGACY_METHODS 0 > #define VNL_CONFIG_THREAD_SAFE 1 These should be the default configured values. Amitha. |
From: Brad K. <bra...@ki...> - 2003-12-19 15:08:49
|
Hello, Since the configuration of vnl_config.h seems to be a popular idea, I've implemented the change. The three options are now in the cache as advanced values with the same defaults: VNL_CONFIG_CHECK_BOUNDS ON VNL_CONFIG_LEGACY_METHODS OFF VNL_CONFIG_THREAD_SAFE ON Peter, I've written the vnl_config.h.in file in a way that should be compatible with configure. You'll need to add its configuration to the autoconf-based build system. -Brad |
From: Peter V. <Pet...@es...> - 2003-12-20 10:55:45
|
> Since the configuration of vnl_config.h seems to be a popular idea, I've > implemented the change. All right! |