Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

binary/inline tensor #96

Open
Knight-X opened this issue Apr 21, 2018 · 16 comments

Comments

6 participants
@Knight-X
Copy link
Member

commented Apr 21, 2018

To remove the dependency on SD card for model parameters, we could compile weight parameter into code and flash onto the chip. We can make those parameters stored in read only flash area and create tensors using const array instead of idxImporter. Any Ideas ?

@Knight-X

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2018

hey, since there are no comments in these days, i would try to do some stuff as draft. Comments welcome.

@mbartling

This comment has been minimized.

Copy link
Member

commented Apr 23, 2018

I think we should still maintain the importer layer.
i.e. something like this, or have an importer factory or something.

class ImporterBase{
 public:
   virtual some_read_write_interface(...)
};
class ImporterIdxSD : public ImporterBase ...
class ImporterIdxConst: public ImporterBase ...
@Knight-X

This comment has been minimized.

Copy link
Member Author

commented Apr 24, 2018

That is cool and it will minimize the complexity of idxloader now. Just we have to consider whether we support idx format or not ? In order to minimize the code size, i prefer to compile the const weight pointer into flash area purely. What do you think ?

@neil-tan

This comment has been minimized.

Copy link
Member

commented Apr 25, 2018

Performance is a major consideration for the binary/inline tensor class. It would be nice if it adapts target devices' native format.

I am trying to think of the reason for keeping the importer here. @mbartling, my guess is we can do more runtime trick this way?

Keep in mind that every tensor also inherits an init and de-init hook from the tensor-base class.

@mbartling

This comment has been minimized.

Copy link
Member

commented Apr 26, 2018

I would still sacrifice the slight read time and have the FormatReaders separate from the PlaceImporters.
For example ImporterSD<IdxReader> myReader(); or ImporterSD<WAVReader>, or ImporterSdRam<ConstReader>.

Block memory reads are way more expensive than adding one more read pointer function call, so having the additional configurability greatly simplifies the code base without any real cost. Additionally, it makes adding Exporters trivial.

@Knight-X what do you think

@neil-tan

This comment has been minimized.

Copy link
Member

commented Apr 26, 2018

I like the idea of having the FormatReaders and PlaceImporters separated. We might be able to make universal tensor importers using polymorphism, Importer(Tensor* t, FmtReader* pipe)

@mbartling Can you clarify what you meant by "Block memory"? Block in the sense of a chunk of memory or busy I/O?

@Knight-X

This comment has been minimized.

Copy link
Member Author

commented Apr 27, 2018

I think that interface is cool. In that case, we could support different format optionally. In your example, you mention ImporterSD.
I just wonder what is "WAV" ? Is this the file format of mbed ?

@neil-tan

This comment has been minimized.

Copy link
Member

commented Apr 27, 2018

wave files, think of it as bmp for sound.

@mbartling

This comment has been minimized.

Copy link
Member

commented Apr 27, 2018

@neil-tan Block memory as grab adjacent bytes. It's partially file system dependent for performance

@neil-tan

This comment has been minimized.

Copy link
Member

commented Apr 29, 2018

To give it a bit of context:

@Knight-X

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2018

I think that clean importer interface is nice, but we need to consider whether it could reduce the sram memory usage in binary tensor. Should we just consider the interface of making binary tensor first ? Then, we could just make importer interface clean and decide what type of tensor should be supported.

@neil-tan

This comment has been minimized.

Copy link
Member

commented Apr 29, 2018

Hey, just spoke with Marcus, looks like the bottomline is having the tensor-data-void-pointer be static-const. This should allow us to keep the model parameters on the flash instead of heap. Creating a new Tensor class is probably the easiest way. It will be read-only. Don't mind having the importer here as long as the interface makes sense and keeps the data in flash.

@marcuschangarm

This comment has been minimized.

Copy link

commented Apr 29, 2018

the bottomline is having the tensor-data-void-pointer be static-const.

Just to clarify, the important part is having the actual array being declared const and not just the pointer.

@Knight-X

This comment has been minimized.

Copy link
Member Author

commented May 2, 2018

I have drafted binary tensor, and i will write test case which is similar to sdtensor test for it. Therefore, I need some feature to transform weights to const pointer in c++, which will be in codegen.

@neil-tan neil-tan moved this from To do to In progress in CNN Demo May 2, 2018

@neil-tan neil-tan added this to WIP in CMSIS-NN Jun 7, 2018

@neil-tan neil-tan moved this from WIP to Done in CMSIS-NN Jun 7, 2018

@neil-tan neil-tan moved this from Done to WIP in CMSIS-NN Jun 7, 2018

@chynphh

This comment has been minimized.

Copy link

commented Sep 19, 2018

uTensor is a amazing job. I want to use it to speed up my model. But I got some trouble.
How can I initialize Tensor with a vector or other something, not idx file?Just like this for
"simple_mnist":

Tensor *data;
data = new Tensor();
std::vector<uint32_t> tmp(784, 0);
data->init(tmp);
get_simple_mnist_ctx(ctx, data);
S_TENSOR pred_tensor = ctx.get("y_pred:0");
ctx.eval();
print_tensor<int>(pred_tensor);

Thank you a lot.

@dboyliao

This comment has been minimized.

Copy link
Member

commented Sep 19, 2018

If you want to initialize tensor with zeros, how about:

uint32_t *w_ptr = data->write<uint32_t>(0, 0);
for (uint32_t i = 0; i < data->getSize(); ++i) {
    *(w_ptr + i) = 0;
}

Or you want sth without a loop?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.