[vsipl++] Matlab IO
Jules Bergmann
jules at codesourcery.com
Tue Jun 27 19:39:13 UTC 2006
Assem Salama wrote:
> Assem Salama wrote:
>> Everyone,
>> Fixed endian stuff.
Assem,
>> +
>> + template <typename T,size_t type_size,bool to_swap_or_not_to_swap>
>> + struct swap_value + { + static void swap(T *d) {d=d;} + };
For our coding convention, struct and class names should begin with a
capital letter: 'Swap_value'
>> +
>> + template <typename T>
>> + struct swap_value<T,2,true>
>> + {
>> + static void swap(T* d)
>> + {
>> + char *p = reinterpret_cast<char*>(d);
>> + std::swap(p[0],p[1]);
>> + }
>> + };
>> +
>> + template <typename T>
>> + struct swap_value<T,4,true>
>> + {
>> + static void swap(T* d)
>> + {
>> + char *p = reinterpret_cast<char*>(d);
>> + std::swap(p[0],p[3]);
>> + std::swap(p[1],p[2]);
>> + }
>> + };
>> +
>> + template <typename T>
>> + struct swap_value<T,8,true>
>> + {
>> + static void swap(T* d)
>> + {
>> + char *p = reinterpret_cast<char*>(d);
>> + std::swap(p[0],p[7]);
>> + std::swap(p[1],p[6]);
>> + std::swap(p[2],p[5]);
>> + std::swap(p[3],p[4]);
>> + }
>> + };
You can wrap the 'Swap_value' class with a function:
template <typename T>
void
swap_value(T& value, bool swap)
{
if (swap)
swap_value<T, sizeof(T), true>::swap(&value);
}
>> +
>> + // swaps an array of values based on a template param
>> + template <typename T>
>> + void swap_array(T *d, vsip::impl::Int_type<1>)
>> + { swap_value<T,sizeof(T),true>::swap(&(d[0])); }
>> + template <typename T>
>> + void swap_array(T *d, vsip::impl::Int_type<2>)
>> + { swap_value<T,sizeof(T),true>::swap(&(d[0]));
>> + swap_value<T,sizeof(T),true>::swap(&(d[1])); }
>> + template <typename T>
>> + void swap_array(T *d, vsip::impl::Int_type<3>)
>> + { swap_value<T,sizeof(T),true>::swap(&(d[0]));
>> + swap_value<T,sizeof(T),true>::swap(&(d[1]));
>> + swap_value<T,sizeof(T),true>::swap(&(d[2])); }
Is there a reason that swap_array takes the array size as a template
parameter?
The pros of passing the size this way are primarily in performance. The
above code will force the compiler to inline the individual calls
instead of performing a loop.
However, the cons are increased code size (both source code and object
code), longer compile times, and limiting the situations where the
function can be called.
Can you change swap_array to take the array size as a regular parameter.
>> +
>> + // swaps the header of a view
>> + template <vsip::dimension_type dim>
>> + void swap_header(view_header<dim> &header, uint16_t endian)
Instead of passing 'endian' to each function and having it decode 'MI'
vs 'IM' to determine if swapping is necessary, it would be better to
have operator>> perform the check and pass a boolean parameter:
void swap_header(..., bool swap_bytes)
This centralizes the knowledge of how matlab files encode endianness
into one place in the code instead of replicating it. It also makes the
function's behavior easier to understand.
>> + {
With the 'swap_value' helper function and the 'swap_bytes' bool
parameter, these become:
swap_value(header.header.type, swap_bytes);
swap_value(header.header.size, swap_bytes);
...
>> + if(endian == ('I' << 8 | 'M') )
>> + {
>> + // swap all fields
>> + swap_value<int32_t,4,true>::swap(&(header.header.type));
>> + swap_value<uint32_t,4,true>::swap(&(header.header.size));
>> +
>> swap_value<int32_t,4,true>::swap(&(header.array_flags_header.type));
>> +
>> swap_value<uint32_t,4,true>::swap(&(header.array_flags_header.size));
>> + swap_value<int32_t,4,true>::swap(&(header.dim_header.type));
>> + swap_value<uint32_t,4,true>::swap(&(header.dim_header.size));
>> +
>> swap_value<int32_t,4,true>::swap(&(header.array_name_header.type));
>> +
>> swap_value<uint32_t,4,true>::swap(&(header.array_name_header.size));
>> + swap_array<uint32_t>(header.dim, vsip::impl::Int_type<dim>());
>> + swap_array<uint32_t>(header.array_flags,
>> vsip::impl::Int_type<2>());
>> + }
>> + }
>> +
>> + // generic reader that allows us to read a generic type and cast to
>> another
>> + + // the read function for real or complex depending of the view
>> that was
>> + // passed in
What does "generic type" mean in the above comment?
A comment like the following would be more clear:
// Read a N-dimensional matlab XXX. The value type of the file object
// is specified by T1 and can be different from the value type of the
// VSIPL++ view. For views of complex data, this function is called
// once for the real subview and once for the imaginary subview.
>> + template <typename T1,
>> + typename ViewT>
>> + void read(std::istream& is,ViewT v,uint16_t endian)
>> + {
>> + vsip::dimension_type const View_dim = ViewT::dim;
>> + vsip::Index<View_dim> my_index;
>> + vsip::impl::Length<View_dim> v_extent = extent(v);
>> + T1 data;
>> + typedef typename ViewT::value_type scalar_type;
>> + typedef void (*fn_type)(T1 *data);
>> + fn_type swap_fn;
>> +
>> + // get num_points
>> + vsip::length_type num_points = v.size();
>> +
>> + // figure out if we need to do endian swaps
>> + if(endian != ('M' << 8 | 'I'))
>> + swap_fn = swap_value<T1,sizeof(T1),true>::swap;
>> + else
>> + swap_fn = swap_value<T1,sizeof(T1),false>::swap;
>> +
>> + // read all the points
>> + for(vsip::index_type i=0;i<num_points;i++) {
>> + is.read(reinterpret_cast<char*>(&data),sizeof(data));
>> + swap_fn(&data);
With 'swap_value' wrapper, this becomes:
swap_value(data, swap_bytes);
>> + put(v,my_index,scalar_type(data));
>> +
>> + // increment index
>> + my_index = vsip::impl::next(v_extent,my_index);
>> + }
>> +
>> + }
>> +
>> +
>> +// operator to read view from matlab file
>> +template <typename T,
>> + typename Block0,
>> + template <typename,typename> class View>
>> +inline
>> +std::istream&
>> +operator>>(
>> + std::istream& is,
>> + Matlab_bin_formatter<View<T,Block0> > mbf)
>> +{
>> + matlab::data_element temp_data_element;
>> + matlab::view_header<vsip::impl::Dim_of_view<View>::dim> m_view;
>> + typedef typename vsip::impl::Scalar_of<T>::type scalar_type;
>> + typedef matlab::Subview_helper<View<T,Block0> > subview;
>> + typedef typename subview::realview_type r_v;
>> + typedef typename subview::imagview_type i_v;
>> + vsip::dimension_type v_dim = vsip::impl::Dim_of_view<View>::dim;
>> +
>> +
>> + // read header
>> + is.read(reinterpret_cast<char*>(&m_view),sizeof(m_view));
After reading the header, this is the place to check 'endian' to
determine if byte swapping is necessary:
bool swap_bytes = (endian != 'M' << 8 | 'I');
>> +
>> + // do we need to swap fields?
Since we already know if it is necessary to swap bytes, the comment
might be:
// Swap header fields if necessary.
>> + matlab::swap_header(m_view,mbf.header.endian);
>> +
>> + // is this complex?
>> + if(vsip::impl::Is_complex<T>::value &&
>> !(m_view.array_flags[0]&(1<<11)))
>> + VSIP_IMPL_THROW(std::runtime_error(
>> + "Trying to read complex matrix into a real matrix"));
>> +
>> +
>> + // is this the same class?
>> + if(!((m_view.array_flags[0] & 0xff) == +
>> (matlab::Matlab_header_traits<sizeof(scalar_type),
>> + std::numeric_limits<scalar_type>::is_signed,
>> +
>> std::numeric_limits<scalar_type>::is_integer>::class_type)
>> + ))
>> + VSIP_IMPL_THROW(std::runtime_error(
>> + "Trying to read a matrix of a different class"));
The error message should mention "view" instead of "matrix":
"Trying to read a view of a different class"
>> +
>> + // do dimensions agree?
>> + if(v_dim == 1) m_view.dim_header.size -= 4; // special case for
>> vectors
>> + if(v_dim != (m_view.dim_header.size/4))
>> + VSIP_IMPL_THROW(std::runtime_error(
>> + "Trying to read a matrix of different dimensions"));
"Trying to read a view with different dimensions"
>> +
>> + for(vsip::dimension_type i=0;i<v_dim;i++)
>> + if(mbf.view.size(i) != m_view.dim[i])
>> + VSIP_IMPL_THROW(std::runtime_error(
>> + "Matrix dimensions don't agree"));
"View dimensions don't agree"
--
Jules Bergmann
CodeSourcery
jules at codesourcery.com
(650) 331-3385 x705
More information about the vsipl++
mailing list