Allow integer types for index variables

[marcsous]

In MappedTensor.m, you could generalize the validation check to allow all integer types (int16, int32, etc.). Line 1590: validateattributes(oRefs, {'numeric'}, {'integer', 'real', 'positive'});

For the validateattributes() function, Matlab's documentation states:

  "The string 'numeric' is an
   abbreviation for the classes uint8, uint16, uint32, uint64,
   int8, int16, int32, int64, single, double."

Works for me. Thanks for creating this file, it's a real improvement over memmapfile.

[marcsous]

Actually let me retract that.... it isn't working properly with uint types. I wonder if you would consider adding support for them transparently? It would help a lot when using MappedTensor as a drop-in replacement for large matrices inside Matlab. I looked in the code a little more but it got a bit hairy for me... Perhaps could be solved (hacked!) by casting all inputs to doubles but your code deserves to be treated properly :)

Here are a few issues I've noticed:

• The class of the returned size is not the same as Matlab's internal function.

X = zeros(uint8([32 32]));
class(size(X))
ans =
double

X = MappedTensor(uint8([32 32]));
class(size(X))
ans =
uint8

• Initialization allow non-float types but then fails when trying to access the array.

X = MappedTensor(uint8([32 32]));
X(:)
Error using mapped_tensor_shim
*** mapped_tensor_shim: Could not allocate data buffer.
Error in MappedTensor>mt_read_data (line 1607)
      tData = hShimFunc('read_all', hDataFile, vnTensorSize, ...
Error in MappedTensor/my_subsref (line 433)
            tfData = mtVar.fRealFactor .* mt_read_data(mtVar.hShimFunc,
            mtVar.hRealContent, S, vnReferencedTensorSize, mtVar.strClass,
            mtVar.nHeaderBytes, mtVar.bBigEndian, mtVar.hRepSumFunc,
            mtVar.hChunkLengthFunc);
Error in MappedTensor/subsref (line 342)
               [varargout{1}] = my_subsref(mtVar, subs); 

• Non-float arguments are not currently supported for indices.

X = MappedTensor(double([32 32]));
X(uint8(20),uint8(20))
Error using MappedTensor>isvalidsubscript (line 1594)
*** MappedTensor: Subscript indices must either be real positive integers or
logicals.
Error in MappedTensor/my_subsref (line 354)
         cellfun(@isvalidsubscript, S.subs);
Error in MappedTensor/subsref (line 342)
               [varargout{1}] = my_subsref(mtVar, subs); 

• After changing line 1590 as in the post above to try to allow int types, the indexing is wrongly calculated.

X = MappedTensor(double([32 32]));
X(:)=1:32^2;
X(double(20),double(20))
ans =
   628
X(uint8(20),uint8(20))
ans =
    20

• I renamed the private function to force it to use pure Matlab code and found a little problem with colon operator.

X = MappedTensor(32,32);
Warning: --- MappedTensor: Could not compile MEX functions.  Using slow matlab
versions. 
> In MappedTensor.MappedTensor>GetMexFunctionHandles at 1940
  In MappedTensor.MappedTensor>MappedTensor.MappedTensor at 258 

X(:)=1:32^2;
X(20,20)
ans =
     0

X(1:32^2)=1:32^2;
X(20,20)
ans =
   628

• With Matlab only code (after renaming the private directory) the overflow value for indices is different.

X = MappedTensor(32,32); % using C mex functions
X(1:32^2)=1:32^2;
X(uint8(20),uint8(20))
ans =
    20
clear
X = MappedTensor(32,32); % NOT using C mex functions
Warning: --- MappedTensor: Could not compile MEX functions.  Using slow matlab
versions. 
> In MappedTensor.MappedTensor>GetMexFunctionHandles at 1940
  In MappedTensor.MappedTensor>MappedTensor.MappedTensor at 258 
X(1:32^2)=1:32^2;
X(uint8(20),uint8(20))
ans =
   255

[marcsous]

OK, so sorry to spam your board! After some digging around it didn't seem so hard so I had a go at the mods. These are the changes made. Everything works with the exception of the colon operators in "slow matlab" mode - I don't know how to fix those but put errors as placeholders.

diff -w old/MappedTensor.m new/MappedTensor.m
288,289c288,289
<          % - Record the original tensor size
<          mtVar.vnOriginalSize = vnTensorSize;
---
>          % - Record the original tensor size -- cast to double to avoid problems
>          mtVar.vnOriginalSize = double(vnTensorSize);
1590c1590
<          validateattributes(oRefs, {'single', 'double'}, {'integer', 'real', 'positive'});
---
>          validateattributes(oRefs, {'numeric'}, {'integer', 'real', 'positive'});
1724a1725
> 
1752,1753c1753,1754
<          % - This dimension was ok
<          cCheckedRefs{nRefDim} = cRefs{nRefDim};
---
>          % - This dimension was ok -- cast to double to avoid problems
>          cCheckedRefs{nRefDim} = double(cRefs{nRefDim});
1762a1764
> 
1824c1826
<    
---
>    %662
1895a1898,1900
>       case 'read_all'
>           error('colon read_all operator is not implemented')
>           
1897a1903,1905
>          
>       case 'write_all'
>           error('colon write_all operator is not implemented')

[DylanMuir]

Hi @marcsous,

I appreciate your feedback, thanks a lot. It will take me a couple of days to look over the code and see how I can best address the issues (especially the colon problem).

Cheers,
Dylan.

[DylanMuir]

By the way, feel free to make a pull request and I'll check out your code edits.

D.

[marcsous]

Hi Dylan,

I'm not sure how to do a pull request... I think I just forked your project instead. Either way you can view the diffs here:

https://github.com/marcsous/MappedTensor/commits/master

I noticed sum() was always returning double matrices so I modified it to work exactly like Matlab version. Should work with the new trailing string arguments too (default, omitnan, includenan) but I don't have a recent version of Matlab to check it on.

Mark

[DylanMuir]

Hi @marcsous, so I should ignore #2 and just work on #3?

[marcsous]

Yes - I'm going to step away from the computer and leave it alone for a while. Thanks, it's really nice code to work with.

[DylanMuir]

Thanks :)

Ok, I’ll get into it as soon as I can.

On 24 Apr 2016, at 4:57 pm, marcsous notifications@github.com wrote:

Yes - I'm going to step away from the computer and leave it alone for a while. Thanks, it's really nice code to work with.

—
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub

[DylanMuir]

Ok, as far as I can tell all these issues are solved.

• Integer creation arguments permitted
• Integer subscripts permitted
• Integer subscripts reference the correct data
• Slow Matlab shim functions support 'read_all' and 'write_all'
• Support for outtype argument when calling sum

@marcsous, please check and see if I've missed anything. Thanks again for your help!

Cheers,
Dylan.

[marcsous]

Maybe a few bugs still there.

You need to call GetMexFunctionHandles before the validation check to avoid an ugly file error.

>> a=MappedTensor(1.5)
Warning: --- MappedTensor/delete: Could not delete temporary file.
       Error: Index exceeds matrix dimensions. 
> In MappedTensor.MappedTensor>MappedTensor.delete at 331 
Error using MappedTensor (line 260)
*** MappedTensor: Error: 'vnTensorSize' must be a positive integer vector. 

Trailing singletons still there.

>> a=MappedTensor(2,1,1);
>> size(a)
ans =
     2     1     1

Complex assign still buggy.

>> a=MappedTensor(1);
>> a(1)=i;
>> a(1)=1+i; a(:)
ans =
   1.0000 + 1.0000i
>> a(1)=0; a(:)
ans =
   0.0000 + 1.0000i

Regarding the fast file creation, I found there is a similar command on windows called fsutil file createnew. Macs may have fallocate (or it can be installed) so I would leave fallocate available for mac.

Other than that, I think it's good to go! I made another fork with just these changes.
https://github.com/marcsous/MappedTensor/commit/9f0d3c0b741aaa2732983cbfea5b26e90a6bebc5

Thanks for taking the time and effort to make these changes.

[DylanMuir]

Hi @marcsous,

Ok, complex assign is a new one. That's not fair :)
I'll check it out.

fsutil file createnew just writes a file full of zeros, which is no better or different that what I was already doing with fwrite.

fallocate is different, but I'm not sure if it's better or not. It's faster because it doesn't zero the data, just defines space for a file. But not the other hand, you then have a "new" tensor that's full of garbage. That doesn't seem desirable, especially if you just convert a real tensor to a complex tensor... Then the complex part will suddenly be non-zero and weird for the whole tensor. That's why I disabled the code (but left it in).

[DylanMuir]

Ok, I can't reproduce the complex assign bug. Could you please verify with the latest version please?

>> a = MappedTensor(1)

a = 
  MappedTensor object, containing: double [1 1].
  Methods

>> a(1) = 1+i; a(:)
ans =
   1.0000 + 1.0000i

>> a(1) = 0; a(:)
ans =
   0.0000 + 0.0000i

[marcsous]

I changed the complex assign code earlier, maybe it got merged in. Sorry I am having trouble getting to learn github so I've deleted and recreated the fork several times as I mess it all up.

I only have Windows XP in a VM but this command takes less than a second from the commandline.

fsutil file createnew myfile.bin 1000000000

When I use the Matlab version (same machine but on the Linux side) it takes a minute. The fallocate is also very fast (less than a second).