mlpack / models

models built with mlpack
https://models.mlpack.org/docs
BSD 3-Clause "New" or "Revised" License
35 stars 40 forks source link

Restructure of the model files #60

Closed Aakash-kaushik closed 3 years ago

Aakash-kaushik commented 3 years ago

There are two things i would like to talk about.

  1. Currently the structure for the models inthe repo is something like this
    - models
    - yolo
    - yolo files
    - darkent 
    - darknet files

    This means that when a user wants to use say models if it was installed which we don't have right now someone would need to write

    // Not sure if there would be models two times, just going by the repo strcture. 
    #include <models/models/resnet/yolo.hpp> 

    I propose to change the structure to something like this:

    - models
    - yolo files
    - darknet files

    so the importing for the user becomes something like

    #include <models/models/yolo.hpp> 
  2. Have a single file that the user can include and have access to all the models without the burden of dragging all the code and using only the code for the model which has been used. I am not sure of how we can do the second one without impacting the build time.
zoq commented 3 years ago

Just to see if I get this right, so this means that we would put the hpp, _impl.hpp files directly into https://github.com/mlpack/models/tree/master/models instead of having them in another folder?

Aakash-kaushik commented 3 years ago

Just to see if I get this right, so this means that we would put the hpp, _impl.hpp files directly into https://github.com/mlpack/models/tree/master/models instead of having them in another folder?

Yes

kartikdutt18 commented 3 years ago

This was intentional, To do two things :

  1. Reduce file size : Its not generally advisable to have a file with very large code. So In the .hpp file we could include multiple implementation files if they don't share code. Example, YOLOv5 and YOLOv1 might not have much in common, so we have an implementation file for v1, v2 and one for v5.
  2. Directory looks cleaner : Rather than having a lot of files for v1, v2 in one directory, we similar files are clubbed in a folder.

Possibility of improvement : Also, we there is a possibility that we build common helper function that will be shared by models of similar type. So we can eventually transition to #include <models/computer_vision/yolo.hpp> to minimize code duplicity as models are added. I feel this will be a better approach to reduce clutter in terms of code and files.

Example : Similar blocks are shared by Darknet and Yolo models, They might also be shared some other models say resnet.hpp, so rather than having them inside darknet.hpp and resnet.hpp, we can have a ComputerVisionModelsHelper (or something), that will be a friend class to these models, and the models can use the above class to add blocks. This also prevents unnecessary exposure of functions. So directory will look like :

models 
 - computer_vision
    - computer_vision_model_helper.hpp
    - Other models
Aakash-kaushik commented 3 years ago

@kartikdutt18 So what i understand is that if you want to have a file like computer_vision_helper.hpp it would be hard to implement something like that afterwards? and should be kept in mind whenver a model is implemented. Though I am a lot more clueless here than what i started with.

mlpack-bot[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! :+1:

mlpack-bot[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! :+1: