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

LightGBM inference result matches input type #2129

Merged
merged 1 commit into from Nov 7, 2022

Conversation

zachgk
Copy link
Contributor

@zachgk zachgk commented Nov 3, 2022

This makes the LightGBM inference return the same as the input type (either float32 or float64) rather than always returning float64 per the API.

In addition, it adds a bit nicer handling for creation from a Buffer in that it will also accept either a FloatBuffer or a DoubleBuffer.

ByteBuffer bb = ByteBuffer.allocateDirect(length * 4);
FloatBuffer wrapped = bb.asFloatBuffer();
for (int i = 0; i < length; i++) {
wrapped.put((float) lightgbmlib.doubleArray_getitem(outBuffer, i));
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 call lightgbm.floatArray_getitem()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The behavior of the lightgbm engine is to always return a double array and this is the utility provided by lightgbm to get from a JNI double[] data structure. So if we want to return a float[], we need to convert it from double to float

This makes the LightGBM inference return the same as the input type (either
float32 or float64) rather than always returning float64 per the API.

In addition, it adds a bit nicer handling for creation from a Buffer in that it
will also accept either a FloatBuffer or a DoubleBuffer.
@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2022

Codecov Report

Base: 72.08% // Head: 71.49% // Decreases project coverage by -0.59% ⚠️

Coverage data is based on head (21db500) compared to base (bb5073f).
Patch coverage: 71.84% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2129      +/-   ##
============================================
- Coverage     72.08%   71.49%   -0.60%     
- Complexity     5126     6315    +1189     
============================================
  Files           473      624     +151     
  Lines         21970    27871    +5901     
  Branches       2351     3009     +658     
============================================
+ Hits          15838    19927    +4089     
- Misses         4925     6480    +1555     
- Partials       1207     1464     +257     
Impacted Files Coverage Δ
api/src/main/java/ai/djl/modality/cv/Image.java 69.23% <ø> (-4.11%) ⬇️
...rc/main/java/ai/djl/modality/cv/MultiBoxPrior.java 76.00% <ø> (ø)
...rc/main/java/ai/djl/modality/cv/output/Joints.java 71.42% <ø> (ø)
.../main/java/ai/djl/modality/cv/output/Landmark.java 100.00% <ø> (ø)
...main/java/ai/djl/modality/cv/output/Rectangle.java 72.41% <0.00%> (ø)
...i/djl/modality/cv/translator/BigGANTranslator.java 21.42% <0.00%> (-5.24%) ⬇️
.../modality/cv/translator/ImageFeatureExtractor.java 0.00% <0.00%> (ø)
.../ai/djl/modality/cv/translator/YoloTranslator.java 27.77% <0.00%> (+18.95%) ⬆️
...modality/cv/translator/wrapper/FileTranslator.java 44.44% <ø> (ø)
...y/cv/translator/wrapper/InputStreamTranslator.java 44.44% <ø> (ø)
... and 558 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@lanking520 lanking520 merged commit 69f94cf into deepjavalibrary:master Nov 7, 2022
@zachgk zachgk deleted the lgbm2 branch November 7, 2022 18:06
zachgk added a commit to zachgk/djl that referenced this pull request Nov 8, 2022
This makes the LightGBM inference return the same as the input type (either
float32 or float64) rather than always returning float64 per the API.

In addition, it adds a bit nicer handling for creation from a Buffer in that it
will also accept either a FloatBuffer or a DoubleBuffer.
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.

None yet

4 participants