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

Uncommenting Bool #42

Merged
merged 6 commits into from
Dec 30, 2024
Merged

Uncommenting Bool #42

merged 6 commits into from
Dec 30, 2024

Conversation

dstarkenburg
Copy link
Contributor

Uncommenting Bool and converting Python's Bool datatype directly to Bool in julia does not seem to cause any issues and actually made the onnx layer 'And' load correctly (at least without broadcasting, but that part is on me).

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.82%. Comparing base (5ed108b) to head (84b722f).
Report is 19 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
- Coverage   88.37%   85.82%   -2.56%     
==========================================
  Files           4        4              
  Lines         387      402      +15     
==========================================
+ Hits          342      345       +3     
- Misses         45       57      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jw3126
Copy link
Owner

jw3126 commented Dec 27, 2024

@dstarkenburg can you bump the patch version?

@GunnarFarneback
Copy link
Collaborator

GunnarFarneback commented Dec 28, 2024

I don't think Python's datatypes have anything to do with it since ONNXRunTime is implemented in C++. But combining https://github.com/onnx/onnx/blob/6fab903f0b25eedb5a9d74c47ab095ee97eef0a6/onnx/onnx.proto3#L618 and the discussion in microsoft/onnxruntime#3689, it seems safe to assume that Boolean tensors should use one byte per element, which Julia's Bool does:

julia> sizeof(Bool)
1

.gitignore Outdated Show resolved Hide resolved
src/versions.jl Outdated Show resolved Hide resolved
@dstarkenburg
Copy link
Contributor Author

Let me know if that is what you meant by 'bump the patch!' (I have very little experience with github releases)

@jw3126 jw3126 merged commit b1b81be into jw3126:main Dec 30, 2024
7 of 10 checks passed
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.

4 participants