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 ReduceMean node #56

Merged
merged 9 commits into from
Nov 3, 2024
Merged

Conversation

tlane-rbx
Copy link
Contributor

Adds support for ReduceMean ONNX layer and adds a .gitignore file.

@kraiskil
Copy link
Owner

Before looking in detail into this, I noticed there are a few other Reduce* nodes. Would it be possible to generalize this similar to what is done in Elementwise* nodes? E.g. ReduceSum is pretty similar to ReduceMean

src/nodes/reducemean.h Outdated Show resolved Hide resolved
Comment on lines 3 to 15
* TEMPLATE node.
* When implementing a new node, use this template
* as a starting point.
*
* This file can be kept as a single .h file with an
* in-header implementation, or it can be split into
* a .h and a .cc file.
*
* Replace all occurances of TEMPLATE in this file.
* Some representative dummy implementation provided.
*
* The functions here are callbacks from the onnx2c
* framework. See node.h for more documentation.
Copy link
Owner

Choose a reason for hiding this comment

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

Comments from the template function left in.

.gitignore Outdated Show resolved Hide resolved
@tlane-rbx
Copy link
Contributor Author

Before looking in detail into this, I noticed there are a few other Reduce* nodes. Would it be possible to generalize this similar to what is done in Elementwise* nodes? E.g. ReduceSum is pretty similar to ReduceMean

Yes, that seems doable. Perhaps I can implement that change in another PR.

@kraiskil
Copy link
Owner

Yes, that seems doable. Perhaps I can implement that change in another PR.

Nice. And yes, no point in cluttering this PR - it already adds value as is.

@jellchou
Copy link

I can not compiled the code after added the reduceMean Op.

@kraiskil
Copy link
Owner

@tlane-rbx I tried to compile it too only now (sorry for the long dealy). There's quite a few loops over std::vector.size() with a signed loop variant. But other than that, this looks good.

@jellchou was that the same problem you were seeing?

@jellchou
Copy link

jellchou commented Oct 28, 2024 via email

@kraiskil
Copy link
Owner

kraiskil commented Nov 3, 2024

Ok, compiles fine now, and looks good. Thanks for the contribution!

@kraiskil kraiskil merged commit 5eafb3b into kraiskil:master Nov 3, 2024
@tlane-rbx tlane-rbx deleted the add-reducemean-node branch November 4, 2024 16:31
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