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

[Relay] Add logical operators #2743

Merged
merged 5 commits into from
Mar 9, 2019
Merged

Conversation

abergeron
Copy link
Contributor

This is a follow-up to #2453, to port the operators that were added to Relay.

The one thing I'm less than certain about is the support_level since I've yet to see some actual documentation about what it means.

@slyubomirsky
Copy link
Contributor

slyubomirsky commented Mar 7, 2019

I would venture to guess (though I am not an authority on it) that a support level of 1 would be appropriate for core, basic operators like the logical connectives

Per Tianqi's point below, consistency with existing logical ops is better

@tqchen
Copy link
Member

tqchen commented Mar 7, 2019

See https://docs.tvm.ai/langref/relay_op.html for support level explaination. I think it should be level 4

@srkreddy1238
Copy link
Contributor

@abergeron look good to me.

Update the support_level as per @tqchen advice. Also update the support_level for NNVM too.

@MarisaKirisame
Copy link
Contributor

Should it be defined using RELAY_REGISTER_BINARY_OP? It mean it work on any dtype.

@abergeron
Copy link
Contributor Author

I've fixed the support level.

I don't think those operators are applicable for all types, but I don't know how to limit the types.

@tqchen tqchen merged commit 2239508 into apache:master Mar 9, 2019
@tqchen
Copy link
Member

tqchen commented Mar 9, 2019

Thanks, @MarisaKirisame @abergeron @srkreddy1238 @slyubomirsky , this is now merged.

We could use a followup PR to update type relation to assert bool types, but since it is not strictly necessary, we can go with the current ver for now

@tqchen
Copy link
Member

tqchen commented Mar 9, 2019

@abergeron please send a followup to update https://docs.tvm.ai/langref/relay_op.html

@MarisaKirisame
Copy link
Contributor

@tqchen I can do it, but I have tons of task next week. I will open a issue. If noone does it next week I will pick it up, does it sound good?

@tqchen
Copy link
Member

tqchen commented Mar 9, 2019

@MarisaKirisame Sounds good, go ahead and open an issue

@abergeron abergeron deleted the relay_logical branch March 9, 2019 21:32
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 9, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 12, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants