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

Rework module structure and packages #469

Merged
merged 6 commits into from
Oct 17, 2022

Conversation

juliabeliaeva
Copy link
Contributor

Current module structure has some problems:

  1. "dataset" module contains two separate groups of classes: preprocessing api and implementation and dataset api and implementation.
  2. "dataset" is not an essential module, and "api" should not have a dependency on it.
  3. Some api classes, like TensorShape or Operation, would fit better in the "api" module and not in "dataset" module.

To fix these problems, this PR introduces new module structure:

  1. "api" has only api classes (including preprocessing api), and no platform-specific code.
  2. "impl" contains implementations, like ImageRecognitionModelBase, preprocessing implementations (including platform-dependent), utility classes, etc. It depends on "api" module.
  3. "dataset" has dataset api and implementations. Depends on "api" and "impl" modules.

This PR also changes some packages, including moving "onnx" module classes to the package org.jetbrains.kotlinx.dl.onnx, and ModelHub and related classes from keras.loaders to just loaders (maybe here other package name like modelhub or hub could be better?). Packages in "tensorflow" module are unchanged, but we may need to refactor them in the future releases.

@ermolenkodev
Copy link
Collaborator

It looks like some tests are not updated, at least these two:

impl/src/jvmTest/kotlin/org/jetbrains/kotlinx/dl/impl/preprocessing/PreprocessingTensorTest.kt
tensorflow/src/test/kotlin/org/jetbrains/kotlinx/dl/api/core/integration/SavedModelTest.kt

@ermolenkodev
Copy link
Collaborator

Not related to the changes, but in this version, I cannot build the project using the command line ./gradlew, only in IntelliJ Idea. Do you know how to use the same toolchain as in IDE when building through the command line?

@ermolenkodev
Copy link
Collaborator

ermolenkodev commented Oct 14, 2022

Can you please elaborate on what should go to the impl package in the new structure? Implementation of interfaces defined in any other package?

Module "api" does not have any module-specific code, and "dataset" is used only for training now.
@juliabeliaeva
Copy link
Contributor Author

Not related to the changes, but in this version, I cannot build the project using the command line ./gradlew, only in IntelliJ Idea. Do you know how to use the same toolchain as in IDE when building through the command line?

No, that was my mistake. I fixed it and it should work now, thank you for pointing it out.

@juliabeliaeva
Copy link
Contributor Author

Can you please elaborate on what should go to the impl package in the new structure? Implementation of interfaces defined in any other package?

Implementations of interfaces from the "api" package, platform-specific code, additional utilities such as CocoUtils or ImageNetPreprocessing, etc.

Initially, I wanted to extract "preprocessing" module, but I noticed that a lot of classes also do not fit into the "api" category and also should be moved into a separate module.

@juliabeliaeva juliabeliaeva merged commit b166d42 into Kotlin:master Oct 17, 2022
@juliabeliaeva juliabeliaeva deleted the module-structure branch October 17, 2022 23:54
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.

2 participants