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

Add thorough test for GMM #153

Merged
merged 3 commits into from
Nov 25, 2019
Merged

Add thorough test for GMM #153

merged 3 commits into from
Nov 25, 2019

Conversation

toelli-msft
Copy link
Contributor

No description provided.

@toelli-msft toelli-msft force-pushed the toelli/test_gmm branch 3 times, most recently from b5e09c6 to f210695 Compare November 19, 2019 13:44
@toelli-msft toelli-msft marked this pull request as ready for review November 19, 2019 13:47
@toelli-msft
Copy link
Contributor Author

This PR is basically the same thing I did in #152 but for GMM instead of ADBench LSTM. I'm adding a C++ wrapper module for the Knossos-generated code so that it can be loaded in Python and tested against a reference implementation.

Copy link
Contributor

@awf awf left a comment

Choose a reason for hiding this comment

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

Generally looks fine -- lots of new files, feels like 1 comment per 10 lines would make sense, but don't force them in -- just crossref to existing versions?

@toelli-msft
Copy link
Contributor Author

I added a few more comments to explain what's going on.

@toelli-msft toelli-msft merged commit f35c0eb into master Nov 25, 2019
@toelli-msft toelli-msft deleted the toelli/test_gmm branch November 25, 2019 12:47
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.

3 participants