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

ti.max() behaves differently with built-in function max #4087

Closed
neozhaoliang opened this issue Jan 21, 2022 · 4 comments · Fixed by #4130
Closed

ti.max() behaves differently with built-in function max #4087

neozhaoliang opened this issue Jan 21, 2022 · 4 comments · Fixed by #4130
Labels
potential bug Something that looks like a bug but not yet confirmed

Comments

@neozhaoliang
Copy link
Contributor

neozhaoliang commented Jan 21, 2022

Example code:

import taichi as ti
ti.init(arch=ti.cpu)


# use built-in max 1.
def main1():
    v = ti.Vector([1, 2, 3])
    w = max(v, -3)
    print(w)  # -3

# use built-in max 2.
def main2():
    v = ti.Vector([1, 2, 3])
    w = max(*v, -3)
    print(w)   # 3

# use ti.max 1.
@ti.kernel
def main3():
    v = ti.Vector([1, 2, 3])
    w = ti.max(v, -3)
    print(w)   # [1, 2, 3]

# use ti.max 2.
@ti.kernel
def main4():
    v = ti.Vector([1, 2, 3])
    w = ti.max(*v, -3) # raise error
    print(w)


main1()
#main2()
#main3()
#main4()

@neozhaoliang neozhaoliang added the potential bug Something that looks like a bug but not yet confirmed label Jan 21, 2022
@lin-hitonami
Copy link
Contributor

This is indeed a problem.
In python scope,

ti.max(4, ti.Vector([3, 4, 5]))  # [4, 4, 5]
max(4, ti.Vector([3, 4, 5]))  # [3, 4, 5]
max(ti.Vector([3, 4, 5]), 4)  # 4

@lin-hitonami
Copy link
Contributor

lin-hitonami commented Jan 24, 2022

ti.max performs broadcast element wise max operation if one of the operand is ti.Vector, and it only receives 2 arguments. Maybe we should replace ti.max with ti.ti_max and let users use it in python scope. cc @strongoier

@lin-hitonami
Copy link
Contributor

lin-hitonami commented Jan 24, 2022

If we change ti.max to max in main4, it also raises error because there is a bug when we process starred expressions when the expression is not a list. I will fix it soon.

@lin-hitonami
Copy link
Contributor

For the accordance in Python scope and Taichi scope, we should also deprecate the use of builtin max and min in Taichi scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
potential bug Something that looks like a bug but not yet confirmed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants