Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

operator: InstanceNormalization operator for wasm and cpu backends #82

Merged
merged 18 commits into from
Feb 15, 2019
Merged

operator: InstanceNormalization operator for wasm and cpu backends #82

merged 18 commits into from
Feb 15, 2019

Conversation

hariharans29
Copy link
Member

This PR is to support InstanceNormalization operator for wasm and cpu backends

@hariharans29 hariharans29 requested a review from fs-eire January 26, 2019 00:58
Copy link
Contributor

@fs-eire fs-eire left a comment

Choose a reason for hiding this comment

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

This is generally good to go.

}

// overriding the checkInputTypes() in the base class because Wasm backend has special type limitations
checkInputTypes(inputs: Tensor[]): boolean {
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 apply the special type check only? I think typescript keyword super can help to call parent class method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now checkInputTypes() in the base class also does some other validation other than type validation and hence we need to copy over that snippet of code as well in this method as this method overrides completely the base method. We can make a change later to make it in such a way that only type check is overridden and other kinds of input validation is still common across base and derived classes (hence prevent code duplication). But there are one or two more ops that require this kind of change. So I prefer to do this separately in another change.

@hariharans29
Copy link
Member Author

hariharans29 commented Feb 7, 2019

Let's hold off on this change for a little bit - I see that onnxruntime implemented the IN op using Eigen and hence I would like to be consistent with that as much as possible. Even though I see no scope for optimization and I feel the current implementation is good enough, I would like to understand if we can directly use onnxruntime code. I will try this tomorrow if possible.

@hariharans29
Copy link
Member Author

Merging this change - if need be we will re-visit later and use Eigen based implementation if perf is an issue for a real model.

@hariharans29 hariharans29 merged commit 3079787 into microsoft:master Feb 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants