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

[pytorch] Allows multiple native jars to package into fat jar #1543

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

frankfliu
Copy link
Contributor

Fixes #1422

Change-Id: I477d85725e84f30a7a79733b16d0224abe83f3ce

Description

Brief description of what this PR is about

  • If this change is a backward incompatible change, why must this change be made?
  • Interesting edge cases to note here

Fixes deepjavalibrary#1422

Change-Id: I477d85725e84f30a7a79733b16d0224abe83f3ce
@frankfliu frankfliu requested a review from zachgk as a code owner March 24, 2022 03:36
@frankfliu
Copy link
Contributor Author

@maziyarpanahi

With this PR, you should be able to package multiple pytorch native library ( > 1.10.2) in a single jar

@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2022

Codecov Report

Merging #1543 (1089b45) into master (bb5073f) will decrease coverage by 1.29%.
The diff coverage is 50.31%.

@@             Coverage Diff              @@
##             master    #1543      +/-   ##
============================================
- Coverage     72.08%   70.79%   -1.30%     
- Complexity     5126     5358     +232     
============================================
  Files           473      500      +27     
  Lines         21970    23375    +1405     
  Branches       2351     2543     +192     
============================================
+ Hits          15838    16549     +711     
- Misses         4925     5550     +625     
- Partials       1207     1276      +69     
Impacted Files Coverage Δ
api/src/main/java/ai/djl/modality/cv/Image.java 69.23% <ø> (-4.11%) ⬇️
...i/djl/modality/cv/translator/BigGANTranslator.java 21.42% <ø> (-5.24%) ⬇️
...odality/cv/translator/BigGANTranslatorFactory.java 33.33% <0.00%> (+8.33%) ⬆️
...nslator/InstanceSegmentationTranslatorFactory.java 14.28% <0.00%> (-3.90%) ⬇️
.../modality/cv/translator/YoloTranslatorFactory.java 8.33% <0.00%> (-1.67%) ⬇️
...i/djl/modality/cv/translator/YoloV5Translator.java 5.69% <0.00%> (ø)
...odality/cv/translator/YoloV5TranslatorFactory.java 8.33% <0.00%> (-1.67%) ⬇️
...pi/src/main/java/ai/djl/ndarray/BytesSupplier.java 54.54% <0.00%> (-12.13%) ⬇️
...pi/src/main/java/ai/djl/repository/Repository.java 83.33% <ø> (ø)
...l/training/loss/SigmoidBinaryCrossEntropyLoss.java 64.00% <0.00%> (ø)
... and 204 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33c29ed...1089b45. Read the comment docs.

@frankfliu frankfliu merged commit a7f025f into deepjavalibrary:master Mar 24, 2022
@frankfliu frankfliu deleted the pt branch March 24, 2022 19:52
@DevinTDHa
Copy link

Hi @frankfliu,

Any information on when pytorch-native-cpu:1.10.2 would be published on maven?

Thanks!

@frankfliu
Copy link
Contributor Author

It should be release together with DJL 0.17.0, we usually release new version in 45-60 days.

@DevinTDHa
Copy link

@frankfliu I've got a question regarding this PR:

We are using sbt to package the libraries for a fat jar, so it will include pytorch-native-cpu with the classifiers osx-x86_64 linux-x86_64 win-x86_64. However as I understand, the corresponding native/lib/pytorch.properties file is the same for all 3 platforms and some merging strategy needs to be used. How would you suggest dealing with this issue? Is there some way to distinguish the different platforms in the properties file?

@lanking520
Copy link
Contributor

@frankfliu I've got a question regarding this PR:

We are using sbt to package the libraries for a fat jar, so it will include pytorch-native-cpu with the classifiers osx-x86_64 linux-x86_64 win-x86_64. However as I understand, the corresponding native/lib/pytorch.properties file is the same for all 3 platforms and some merging strategy needs to be used. How would you suggest dealing with this issue? Is there some way to distinguish the different platforms in the properties file?

From design point of view, you should not bundle three of the classifier (for Mac OS, Linux, Windows) into a single jar. Each OS should have their own jars. Fat jar is used mostly on the Spark related works (mostly Linux based). DJL is using the specific OS to determine which native jar to load

@maziyarpanahi
Copy link

Hi @lanking520

I think the question is regarding this discussion: #1422 and more precisely this conclusion that something needs to happen to allow multiple CPU operating systems to be packaged all together: #1422 (comment)

The problem with Apache Spark is that spark-shell and spark-submit don't support classifier in SBT. So it entirely discards that part and just randomly downloads the first one that it sees (which is always for the wrong operating system)

To go around this issue in Apache Spark, we package all 3 operating systems on the CPU into 1 fat jar. Obviously, the size of the package and the final Fat JAR will be larger than it should be, but it allows us to use that dependency that has a classifier on all operating systems, all machines, and all possible situations (spark-shell, spark-submit, etc.) - we do this with tensorflow-java as well since they use classifier too.

@frankfliu
Copy link
Contributor Author

@DevinTDHa

Currently we don't have a good way to avoid the pytorch.properties conflict for fatjar.
I create a project that allow you load class from .jar file in a fatjar: https://github.com/frankfliu/junkyard/tree/master/jarjar

Please take a look and see if that works for you.

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.

6 participants