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

ONNXRunTime.jl has no bool type #112

Open
dstarkenburg opened this issue Dec 11, 2024 · 5 comments
Open

ONNXRunTime.jl has no bool type #112

dstarkenburg opened this issue Dec 11, 2024 · 5 comments

Comments

@dstarkenburg
Copy link
Contributor

Hello!

Im trying to implement the ONNX layer 'And' which takes two Boolean tensors in and uses logical and elementwise and returns a single Boolean tensor.

Unfortunately, unless I have coded this test set wrong, there is currently no implementation in ONNXRunTime.jl for tensor type bool (why is it commented out?).

Here is my test code.
src/ops.jl:

function and(A, B)
    # Numpy type broadcasting
    if (size(A) != size(B))
        A = A'
    end
    return A .&& B
end

test/saveload.jl:

    @testset "And" begin
        A = [0 1 0; 1 1 0; 0 1 1; 1 1 1]
        A = Matrix{Bool}(A)
        B = [1 0 0; 1 0 0; 0 0 1; 1 1 0]
        B = Matrix{Bool}(B)
        ort_test(ONNX.and, A, B)

        # Test implementation for Numpy-type broadcasting
        C = [1 0 0 1; 1 0 0 1; 0 0 1 0]
        C = Matrix{Bool}(C)
        ort_test(ONNX.and, A, C)

    end

load.jl and save.jl were omited but if you need them to reproduce the issue, they can be found here.

Here is the error I get during testing.
image

@dfdx
Copy link
Collaborator

dfdx commented Dec 11, 2024

I guess And caused some problems and had too little priority at the moment of writing that piece of ONNXRunTime.jl. At least, I had similar shortcomings in my own code for this reason 🤪

If you are sure that your implementation works, correctly, it's perfectly fine to make a PR and add a broken test with a comment linking to this issue.

It may also be worth to raise a corresponding issue in ONNXRunTime.jl repo for visibility.

@dstarkenburg
Copy link
Contributor Author

Based on the test sets of my other four PRs, I have little to no faith in the first draft of this code and usually rely on these tests to truly verify my solution, haha! In theory, the code works exactly as described in ONNX 'And' Documentation; however, I cannot tell whether the tensors are of type Bool, Uint8, or bit vectors.
image
I assumed that tensor(bool) would simply be Bool in Julia, if this isn't correct then this code will not work in its current state.

@dfdx
Copy link
Collaborator

dfdx commented Dec 12, 2024

Do I understand it right that you can write an ONNX graph (file) with And operation and boolean array, but can't make sure that it is correct because ONNXRunTime.jl doesn't support booleans? If so, would Python implementation of onnxruntime work as a one-time check?

@dstarkenburg
Copy link
Contributor Author

Yes, I ONNX.jl will compile and run but the testsets will not. Is ort_test relatively easy to run on Python? (Assuming it exports the layer, compare the output of the Julia function and the onnx layer, and returns the comparison?)

@dfdx dfdx changed the title ONNXRunTime.jl has no ONNXRunTime.jl has no bool type Dec 13, 2024
@dfdx
Copy link
Collaborator

dfdx commented Dec 13, 2024

Implementing ort_test() in Python is relatively straightforward, but to put it to the CI/CD and local tests, you will need to bring in PythonCall and require users to have Python installed on their system, which is quite a lot of work for everyone. For this reason, I'd suggest to test And operation manually using Python and then add @test_broken to this repo. This way you will ensure that the operation in its current state is implemented correctly, while the broken test will indicate when ONNXRunTime gets support for boolean tensors.

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

No branches or pull requests

2 participants