Skip to content

Conversation

@Infinoid
Copy link
Contributor

This fixes a few things related to the -i (input file) parameter of the command line tool.

Fixes:

  1. The TensorBase::TensorBase() constructor sanity checks the number of format mode types, but this sanity check happens too late: TensorStorage::Content() segfaults before the check happens.
  2. CLI tool doesn't reject input files with tensor names not mentioned in the expression
  3. CLI tool doesn't create default formats of the correct sizes when loading tensors

These are fixed by:

  1. copying the check into TensorStorage::Content.
  2. pre-parsing the expression and making sure the tensor name is in there.
  3. creating a ModeFormat vector of the right length, rather than the previous default (Dense).

There isn't a good framework in place to test the command-line tool, so I tested this by hand. Details below.

@Infinoid
Copy link
Contributor Author

Infinoid commented Jan 19, 2021

Here are the cases I tested, before and after the patch.

Normal operation, borrowing the command line from #366.
bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -f=M:dd -time

before:
M file read: 0.101974 ms
M pack:      74.9754 ms
M size: (3 x 3), 80 bytes

Compile:  82.7933 ms
Assemble: 0.061156 ms
Compute:  0.029866 ms

after:
M file read: 0.133323 ms
M pack:      76.4758 ms
M size: (3 x 3), 80 bytes

Compile:  79.9751 ms
Assemble: 0.059453 ms
Compute:  0.028735 ms

Normal operation without specifying a format.
bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -time

before:
zsh: segmentation fault  bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -time

after:
M file read: 0.076466 ms
M pack:      74.4324 ms
M size: (3 x 3), 80 bytes

Compile:  80.9513 ms
Assemble: 0.057279 ms
Compute:  0.030177 ms

Error reporting of loads with invalid tensor names.
bin/taco "A(i,j) = M(i,j)" -i=N:foo.mtx -time

before:
zsh: segmentation fault  bin/taco "A(i,j) = M(i,j)" -i=N:foo.mtx -time

after:
Error: Cannot load 'foo.mtx': no tensor 'N' found in expression

Usage: taco <index expression> [options]
[snip --help message]

Same as above, but with an explicit format specified.

bin/taco "A(i,j) = M(i,j)" -i=N:foo.mtx -f=N:dd -time

before:
N file read: 0.181946 ms
N pack:      76.5303 ms
N size: (3 x 3), 80 bytes

after:
Error: Cannot load 'foo.mtx': no tensor 'N' found in expression

Usage: taco <index expression> [options]
[snip --help message]

Note: the prior behavior in the above case was to read the file, and then not execute anything, and not report any errors.

Specified format is too short.
bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -f=M:d -time

before:
zsh: segmentation fault  bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -f=M:d -time


after:
terminate called after throwing an instance of 'taco::TacoException'
  what():  Error at /home/infinoid/workspace/taco/git/src/storage/storage.cpp:35 in Content:
 The number of format mode types (1) must match the tensor order (2).
zsh: abort      bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -f=M:d -time

Default format is too short. (1d index expression does not match 2d data file.)
bin/taco "A(i) = M(i)" -i=M:foo.mtx -time

before:
zsh: segmentation fault  bin/taco "A(i) = M(i)" -i=M:foo.mtx -time

after:
terminate called after throwing an instance of 'taco::TacoException'
  what():  Error at /home/infinoid/workspace/taco/git/src/storage/storage.cpp:35 in Content:
 The number of format mode types (1) must match the tensor order (2).
zsh: abort      bin/taco "A(i) = M(i)" -i=M:foo.mtx -time

Specified format is too long.
bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -f=M:ddd -time

before:
terminate called after throwing an instance of 'taco::TacoException'
  what():  Error at /home/infinoid/workspace/taco/git/src/tensor.cpp:107 in TensorBase:
 The number of format mode types (3) must match the tensor order (2).
zsh: abort      bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -f=M:ddd -time

after:
terminate called after throwing an instance of 'taco::TacoException'
  what():  Error at /home/infinoid/workspace/taco/git/src/storage/storage.cpp:35 in Content:
 The number of format mode types (3) must match the tensor order (2).
zsh: abort      bin/taco "A(i,j) = M(i,j)" -i=M:foo.mtx -f=M:ddd -time

Default format is too long. (3d index expression does not match 2d data file.)
bin/taco "A(i,j,k) = M(i,j,k)" -i=M:foo.mtx -time

before:
zsh: segmentation fault  bin/taco "A(i,j,k) = M(i,j,k)" -i=M:foo.mtx -time

after:
terminate called after throwing an instance of 'taco::TacoException'
  what():  Error at /home/infinoid/workspace/taco/git/src/storage/storage.cpp:35 in Content:
 The number of format mode types (3) must match the tensor order (2).
zsh: abort      bin/taco "A(i,j,k) = M(i,j,k)" -i=M:foo.mtx -time

[edit: re-ran a couple of tests to reflect the final version of the error message.]

@stephenchouca stephenchouca merged commit fda53cd into tensor-compiler:master Jan 20, 2021
@Infinoid Infinoid deleted the fix-tensor-loading branch January 21, 2021 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants