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.

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

Refactoring of the preprocessing DSL (#416) #425

Merged
merged 14 commits into from
Aug 24, 2022
Merged

Refactoring of the preprocessing DSL (#416) #425

merged 14 commits into from
Aug 24, 2022

Conversation

ermolenkodev
Copy link
Collaborator

No description provided.

@ermolenkodev ermolenkodev linked an issue Aug 16, 2022 that may be closed by this pull request
Add preprocessing API for android Bitmaps #416
Closed
@juliabeliaeva
Copy link
Contributor

juliabeliaeva commented Aug 16, 2022 •
edited
Loading

Wouldn't it be sufficient to just generify ImagePreprocessor interface? We'd also get the same-looking preprocessing for jvm and android.

juliabeliaeva
juliabeliaeva reviewed Aug 16, 2022
View reviewed changes
dataset/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessor/image/CenterCrop.kt Outdated Show resolved Hide resolved
dataset/src/commonMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessing/Preprocessor.kt Outdated Show resolved Hide resolved
dataset/src/commonMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessing/Preprocessor.kt Outdated Show resolved Hide resolved
dataset/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessor/Preprocessing.kt Outdated Show resolved Hide resolved
dataset/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessor/Preprocessing.kt Outdated Show resolved Hide resolved
dataset/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessor/Preprocessing.kt Outdated Show resolved Hide resolved
dataset/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessor/Preprocessing.kt Outdated Show resolved Hide resolved
dataset/src/androidMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessing/Preprocessing.kt Outdated Show resolved Hide resolved
@juliabeliaeva juliabeliaeva mentioned this pull request Aug 16, 2022
Merged
@juliabeliaeva
Copy link
Contributor

Wouldn't it be sufficient to just generify ImagePreprocessor interface? We'd also get the same-looking preprocessing for jvm and android.

Something like this: master...juliabeliaeva:wip/preprocessing-api

@ermolenkodev ermolenkodev marked this pull request as ready for review August 18, 2022 16:29
@juliabeliaeva
Copy link
Contributor

Would have been better to have separate PR for android implementation.

@juliabeliaeva
Copy link
Contributor

juliabeliaeva commented Aug 18, 2022 •
edited
Loading

We definitely should include more explanation of the changes into the resulting commit message, since we are basically having a new DSL here. Also let's mention in the message that we moved from ImageShape to TensorShape and that getFinalShape now calculates correct shape for operations such as Transpose (previous implementation did not take into account any tensor operations at all). And changes to the save operation should be described.

juliabeliaeva
juliabeliaeva reviewed Aug 18, 2022
View reviewed changes
docs/transfer_learning.md Show resolved Hide resolved
...src/androidMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessing/ConvertToFloatArray.kt Outdated Show resolved Hide resolved
...src/androidMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessing/ConvertToFloatArray.kt Outdated Show resolved Hide resolved
...src/androidMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessing/ConvertToFloatArray.kt Outdated Show resolved Hide resolved
...src/androidMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessing/ConvertToFloatArray.kt Outdated
)
imgData.rewind()
val stride = input.width * input.height
val bmpData = IntArray(stride.toInt())
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't stride already Int?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will fix it in separate PR with operations for an Android platform

...kotlin/examples/transferlearning/modelhub/vgg16/Example_5_VGG16_additional_training_noTop.kt Outdated Show resolved Hide resolved
onnx/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/api/dataset/preprocessor/Transpose.kt Outdated Show resolved Hide resolved
onnx/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/api/dataset/preprocessor/Transpose.kt Show resolved Hide resolved
tensorflow/src/main/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessor/Sharpen.kt Show resolved Hide resolved
...src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/dataset/preprocessor/image/ImageOperationBase.kt Outdated Show resolved Hide resolved
@juliabeliaeva
Copy link
Contributor

Let's add tests for the android operations and for new getFinalShape implementations.

ermolenkodev and others added 13 commits August 22, 2022 17:27
@ermolenkodev @juliabeliaeva
Add new preprocessing DSL (Kotlin#416)
7f14d27 
New preprocessing API implies that preprocessing operations are defined as functional
objects with Input and Output types defined as generics.
Operations can be composed together to form a pipeline. The output of the operation
is the input of the next operation. At the end of the pipeline, the output of the
last operation is the pipeline's output.

The approach when the pipeline is defined as a sequence of operations has the
following advantages:
 - it's more flexible and supports the composition of operations with different
   input and output types.
 - user explicitly specifies conversions between types during the pipeline definition. E.g. there is an Operation that converts from [BufferedImage] to [FloatArray].
 - allows adding support of augmentations in the future without changing the API.

* Remove imagePreprocessing and tensorPreprocessing DSL blocks

* User explicitly convert BufferedImage to FloatArray

* Replace ImageShape with TensorShape

* getOutputShape method now implemented for tensor operations
  and now calculates correct shape for operations such as Transpose

* Update tests and examples with new API

Co-authored-by: Julia Beliaeva <Julia.Beliaeva@jetbrains.com>
@ermolenkodev
Add documentation for the Operation interface (Kotlin#416)
ac72b5d
@ermolenkodev
Fix Convert.getOutputShape method (Kotlin#416)
f60ab47 
* Now method takes requested color mode into account when computing output shape

* Add tests for output shape calculation
@ermolenkodev
Fix getOutputShape for BufferedImage operations (Kotlin#416)
06a2182 
* Add support for unknown dimension in input TensorShape

* Add BufferedImage.getTensorShape extension function
@ermolenkodev
Fix convertation of TensorShape to ImageShape (Kotlin#416)
446dd57 
* Dimensions that are -1 in TensorShape are converted to null in ImageShape.
@ermolenkodev
Add documentation for TensorShape.toImageShape function (Kotlin#416)
91600ef
@ermolenkodev
Optimize imports in modified modules (Kotlin#416)
aa09330
@ermolenkodev
Fix resizeNoInputShape test (Kotlin#416)
b261c44
@ermolenkodev
Minor fixes in examples (Kotlin#416)
529ddb0 
* Remove useless toInt casts

* Fix ImageProessing example

* Remove commented code
@ermolenkodev
Fix Transpose operation (Kotlin#416)
b8b5f51 
* Fix output shape calculation for Transpose operation

* Add test for an output shape calculation
@ermolenkodev
Add FloatArrayOperation base class (Kotlin#416)
c1eb3e3 
* Add FloatArrayOperation base class

* Revert wrong removal of toInt casts in examples
@ermolenkodev @juliabeliaeva
Refactor ImageSave functionality (Kotlin#416)
02fb056 
* Remove ImageOperationBase

* Remove ImageSaver and Operation.save extension function

* Introduce Operation.onResult function which can be used instead of Operation.save
  for saving intermediate results of preprocessing operations

* Add an explitit type declaration for pipeline function

Co-authored-by: Julia Beliaeva <Julia.Beliaeva@jetbrains.com>
@ermolenkodev
Update documentation (Kotlin#416)
76d90cb
@ermolenkodev ermolenkodev changed the title WIP implementation of preprocessing api for Android (#416) Refactoring of the preprocessing DSL (#416) Aug 23, 2022
juliabeliaeva
juliabeliaeva approved these changes Aug 23, 2022
View reviewed changes
dataset/src/jvmMain/kotlin/org/jetbrains/kotlinx/dl/dataset/OnHeapDataset.kt Outdated
import java.io.File
import java.io.IOException
import java.nio.FloatBuffer
import java.nio.file.Files
import java.nio.file.Path
import kotlin.math.truncate
import kotlin.random.Random
import kotlin.streams.toList
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't remove this import, it does not compile without it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, for some reason, tests were green in IntelliJ. I added import back and ran tests using the Gradle command line. Now seems fine

@ermolenkodev
Fix tests (Kotlin#416)
70cf751 
* Fix broken import in OnHeapDataset
@ermolenkodev ermolenkodev merged commit 5a5d385 into Kotlin:master Aug 24, 2022
This was referenced Sep 21, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliabeliaeva juliabeliaeva juliabeliaeva approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add preprocessing API for android Bitmaps
2 participants
@ermolenkodev @juliabeliaeva