-
Notifications
You must be signed in to change notification settings - Fork 655
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
Gradle Kotlin script plus other stuff #3167
Conversation
# Conflicts: # engines/pytorch/pytorch-jni/build.gradle # engines/pytorch/pytorch-native/build.gradle # gradle.properties # gradle/wrapper/gradle-wrapper.properties
docs/README.md
Outdated
@@ -63,16 +63,16 @@ Note: when searching in JavaDoc, if your access is denied, please try removing t | |||
## Extensions and utilities | |||
|
|||
- [Android support](../android/README.md) | |||
- [Audio support](../extensions/audio/README.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need to rename extensions folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the typesafe project accessor clashes with a getter called extensions
, so we either rename that directory or we disable the typesafe accessor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's disable this feature for now. Since this project is experimental, we can create a ticket to gradle and see if they can adda flag to change "extension" folder name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's focus on gradle changes in this PR. Would you please remove these Kotlin tests?
Absolutely, sorry, I already meant to do that, but I forgot |
# Conflicts: # extension/tokenizers/src/main/python/arg_parser.py # extension/tokenizers/src/main/python/djl_converter/__init__.py # extension/tokenizers/src/main/python/djl_converter/arg_parser.py # extension/tokenizers/src/main/python/djl_converter/fill_mask_converter.py # extension/tokenizers/src/main/python/djl_converter/huggingface_converter.py # extension/tokenizers/src/main/python/djl_converter/huggingface_models.py # extension/tokenizers/src/main/python/djl_converter/metadata.py # extension/tokenizers/src/main/python/djl_converter/model_zoo_importer.py # extension/tokenizers/src/main/python/djl_converter/question_answering_converter.py # extension/tokenizers/src/main/python/djl_converter/safetensors_convert.py # extension/tokenizers/src/main/python/djl_converter/sentence_similarity_converter.py # extension/tokenizers/src/main/python/djl_converter/shasum.py # extension/tokenizers/src/main/python/djl_converter/text_classification_converter.py # extension/tokenizers/src/main/python/djl_converter/token_classification_converter.py # extension/tokenizers/src/main/python/djl_converter/zip_utils.py # extension/tokenizers/src/main/python/fill_mask_converter.py # extension/tokenizers/src/main/python/huggingface_converter.py # extension/tokenizers/src/main/python/huggingface_models.py # extension/tokenizers/src/main/python/metadata.py # extension/tokenizers/src/main/python/model_zoo_importer.py # extension/tokenizers/src/main/python/question_answering_converter.py # extension/tokenizers/src/main/python/safetensors_convert.py # extension/tokenizers/src/main/python/sentence_similarity_converter.py # extension/tokenizers/src/main/python/setup.py # extension/tokenizers/src/main/python/shasum.py # extension/tokenizers/src/main/python/text_classification_converter.py # extension/tokenizers/src/main/python/token_classification_converter.py # extension/tokenizers/src/main/python/zip_utils.py # extensions/tokenizers/src/main/python/arg_parser.py # extensions/tokenizers/src/main/python/fill_mask_converter.py # extensions/tokenizers/src/main/python/huggingface_converter.py # extensions/tokenizers/src/main/python/huggingface_models.py # extensions/tokenizers/src/main/python/metadata.py # extensions/tokenizers/src/main/python/model_zoo_importer.py # extensions/tokenizers/src/main/python/question_answering_converter.py # extensions/tokenizers/src/main/python/safetensors_convert.py # extensions/tokenizers/src/main/python/sentence_similarity_converter.py # extensions/tokenizers/src/main/python/shasum.py # extensions/tokenizers/src/main/python/text_classification_converter.py # extensions/tokenizers/src/main/python/token_classification_converter.py # extensions/tokenizers/src/main/python/zip_utils.py # gradle.properties
I re-sync'ed with the upstream and re-ordered the versions in |
settings.gradle.kts
Outdated
@@ -0,0 +1,22 @@ | |||
rootProject.name = "djl" | |||
|
|||
val reserved = listOf("gradle", "buildSrc") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer list all project here, it can also treat as an project index file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer list all project here
Ok, as you wish
it can also treat as an project index file.
What do you mean, exactly?
# | ||
# This file has been generated by Gradle and is intended to be consumed by Gradle | ||
# | ||
[metadata] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is generated file, do we need to check in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was generated, but then I modified to fill in the needed libraries, I'll clean up that original comment
gradle.properties
Outdated
mxnet_version=1.9.1 | ||
pytorch_version=2.2.2 | ||
tensorflow_version=2.10.1 | ||
#djl_version=0.28.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where these version will be defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the libs.versions.toml
above :)
# Conflicts: # engines/pytorch/pytorch-engine/build.gradle # extension/tokenizers/src/main/python/djl_converter/model_converter.py
Addressed last wishes plus re-sync |
I really appreciate your contribution. But I ran into several failures:
|
fixed integration:compileJava compilerArgs removal
# Conflicts: # integration/build.gradle
Still facing a few issues:
|
the sourceCompatibility issue is fixed now, I switched from the toolchain to the error we are seeing now it's another one, which I'm gonna hunt down next |
so, I can reproduce locally interesting to notice, I ran same error:
I ran the test one another time and now I get
|
running |
Ok, I fixed the first error now I have both local and CI crashing with the same last error above |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #3167 +/- ##
============================================
- Coverage 68.41% 63.53% -4.89%
+ Complexity 7027 6451 -576
============================================
Files 697 697
Lines 32759 32790 +31
Branches 3406 3416 +10
============================================
- Hits 22413 20833 -1580
- Misses 8737 10380 +1643
+ Partials 1609 1577 -32 ☔ View full report in Codecov by Sentry. |
9a25da0
to
ace47ca
Compare
Description
This PR convert Gradle Groovy scripts to Kotlin, which is also recommended by Gradle, being a statically typed language
this change is backward compatible
Interesting edge cases to note here:
allprojects
andsubprojects
), which add complexity and burden, preventing also optimizations, like parallel project builds and configuration on demand (more here and here)buildSrc
to abstract imperative logic and share logic via pre-compiled script pluginsgradle/libs.versions.toml
There a couple of
TODO
s which I still have to figure it out, but the 95% work is done and there for you to start looking at it so that I can already get some early feedbacks from you, folksAlso, why are you disabling the Gradle Metadata generation?