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.

Sign up for GitHub

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

Jump to bottom

Issue #2693 Implement PtNDArrayEx.multiBoxPrior with validation #2715

Merged

frankfliu merged 5 commits into deepjavalibrary:master from juliangamble:ptndarrayex_multiboxprior

Aug 14, 2023

Contributor

juliangamble commented Jul 15, 2023 •

edited

Loading

Description

The big goal here is to implement PtNDArrayEx.multiBoxPrior() (currently throws NotImplementException) so that you can run PikachuTraining with Pytorch and so you can run SSD Training on M1 Macs.

This is discussed in: #2693

The big idea is to prove this works by writing matching unit tests for MxNDAarrayEx.multiBoxPrior() and comparing the results.

The implementation for this is derived from:
https://github.com/apache/mxnet/blob/master/src/operator/contrib/multibox_prior.cc
and
https://github.com/apache/mxnet/blob/a720b15b5fa011a9610dfaeabf0792443c6abec5/src/operator/contrib/multibox_prior-inl.h#L116

This implementation is based on implementing the unit test on the C++ side (see unit test in pull request there) and comparing the steps internally.


          Issue deepjavalibrary#2693 Implement PtNDArrayEx.multiBoxPrior with v…

67324b6

…alidation

juliangamble requested review from zachgk, frankfliu and a team as code owners

July 15, 2023 07:09

juliangamble mentioned this pull request

Is it possible to train a PyTorch SSD model on an M1 Mac - or is this not yet implemented? PtNDArrayEx.multiBoxPrior(PtNDArrayEx.java:697) UnsupportedOperationException: Not implemented #2693

Open

frankfliu suggested changes

View reviewed changes

engines/mxnet/mxnet-engine/src/test/java/ai/djl/mxnet/engine/MxNDArrayExTest.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/test/java/ai/djl/pytorch/engine/PtNDArrayExTest.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Show resolved Hide resolved


          pull request feedback

f64910f

frankfliu reviewed

View reviewed changes

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated Show resolved Hide resolved


          Issue deepjavalibrary#2693 Implement PtNDArrayEx.multiBoxPrior with v…

1cbf2ca

…alidation

frankfliu reviewed

View reviewed changes

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated


		NDManager ndManager = array.getManager().getParentManager();

		Float stepX = steps.get(1);

Contributor

frankfliu Jul 25, 2023

Suggested change

      
                    Float stepX = steps.get(1);
          
                    float stepX = steps.get(1);

Contributor Author

juliangamble Jul 25, 2023

Thankyou. This has been updated.

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated

+                      NDManager ndManager = array.getManager().getParentManager();
+                      Float stepX = steps.get(1);
+                      Float stepY = steps.get(0);

Contributor

frankfliu Jul 25, 2023

Suggested change

      
                    Float stepY = steps.get(0);
          
                    float stepY = steps.get(0);

Contributor Author

juliangamble Jul 25, 2023

Thankyou. This has been updated.

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java Outdated

@@ @@ -694,7 +694,54 @@ public NDList multiBoxPrior( @@
                           List<Float> steps,
                           List<Float> offsets,
                           boolean clip) {
-                      throw new UnsupportedOperationException("Not implemented");
+                      NDManager ndManager = array.getManager().getParentManager();

Contributor

frankfliu Jul 25, 2023

Suggested change

      
                    NDManager ndManager = array.getManager().getParentManager();
          
                    NDManager ndManager = array.getManager();

Contributor Author

juliangamble Jul 25, 2023

Thankyou. This has been updated.

engines/pytorch/pytorch-engine/src/main/java/ai/djl/pytorch/engine/PtNDArrayEx.java

+                              }
+                          }
+                      }
+                      NDArray ndArray = ndManager.create(out).expandDims(0);

Contributor

frankfliu Jul 25, 2023

I didn't look into your algorithm, but seems the final output is different from MXNet. Can you debug why it's different?

Contributor Author

juliangamble Jul 25, 2023

There is some discussion of this here:
#2693

I'd love to debug it on the mxnet side, but this is non-trivial. I'm working on a method to do so, but would appreciate some pointers.

Contributor Author

juliangamble Aug 5, 2023

Thanks @frankfliu I found the bug and put in a fix.

For me the first element in the result is -0.09902344 in my implementation and mxnet.
From your perspective is this now returning the correct values?


          [PR-2715] Feedback

f0dae63

codecov-commenter commented Jul 25, 2023 •

edited

Loading

Codecov Report

Patch coverage: 54.72% and project coverage change: +0.08% 🎉

Comparison is base (bb5073f) 72.08% compared to head (0556841) 72.17%.
Report is 865 commits behind head on master.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2715      +/-   ##
============================================
+ Coverage     72.08%   72.17%   +0.08%     
- Complexity     5126     7034    +1908     
============================================
  Files           473      698     +225     
  Lines         21970    31320    +9350     
  Branches       2351     3234     +883     
============================================
+ Hits          15838    22606    +6768     
- Misses         4925     7171    +2246     
- Partials       1207     1543     +336

Files Changed	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% <ø> (ø)`
.../main/java/ai/djl/modality/cv/output/Landmark.java	`100.00% <ø> (ø)`
...djl/modality/cv/transform/RandomFlipLeftRight.java	`25.00% <0.00%> (-25.00%)`	⬇️
...djl/modality/cv/transform/RandomFlipTopBottom.java	`25.00% <0.00%> (-25.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%)`	⬆️
...ain/java/ai/djl/modality/cv/util/NDImageUtils.java	`67.10% <0.00%> (+7.89%)`	⬆️
api/src/main/java/ai/djl/modality/nlp/Decoder.java	`63.63% <ø> (ø)`
... and 226 more

... and 368 files with indirect coverage changes

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


          [feedback] Fixing code based on feedback

frankfliu approved these changes

View reviewed changes

Contributor

frankfliu left a comment

Thanks so much for your contribution.

frankfliu merged commit 5de587c into deepjavalibrary:master

5 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet