Skip to content

Quartus Embedding Layer #548

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

Merged
merged 3 commits into from
Jun 3, 2022
Merged

Conversation

bo3z
Copy link
Contributor

@bo3z bo3z commented May 13, 2022

Added support for Embbeding layer in Quartus, similar to work in this commit: 5040f75#diff-7dd4e5f5df1fd1a2cf2974ce0982818da375ff46be0eda702ba60b4f6188d928

Will be useful once work on SimpleRNN and LSTM is completed.

@bo3z bo3z requested a review from thesps May 13, 2022 09:28
@bo3z bo3z requested a review from vloncar May 31, 2022 12:49
#ifndef NNET_BATCHNORM_STREAM_H_
#define NNET_BATCHNORM_STREAM_H_

namespace nnet {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a stub that raises an exception while we still don't have the implementation? Otherwise the compilation fails with a cryptic message (cannot substitute template argument or something like that). Or does that require the more streaming infrastructure in Quartus (#557)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should never be invoked, as soon as Quartus attempts to use io_stream an exception is thrown from Python (although #557 attempts to add infrastructure for streaming, which means for some layers we should throw exceptions and for some not - this should be handled by Python and can be a part of the streaming PR)

DenseEmbedding:
#pragma unroll
for (int i = 0; i < CONFIG_T::n_out; i++) {
res[j * CONFIG_T::n_out + i] = embeddings[data[j].to_uint() * CONFIG_T::n_out + i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me, why is the to_uint() needed here?

Copy link
Contributor Author

@bo3z bo3z Jun 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data[j] is a fixed point number by default, without to_uint() compilation will fail as ac_fixed and int are not interchangeable - may be allowed with the -fpermissive flag, not sure.

@bo3z bo3z force-pushed the quartus-embedding branch from f8e0416 to 17a3160 Compare June 2, 2022 15:18
@vloncar vloncar merged commit 881efea into fastmachinelearning:master Jun 3, 2022
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