neuronets / trained-models

Trained TensorFlow models for 3D image processing
https://neuronets.dev/trained-models
22 stars 15 forks source link

feat(Model): Add model through issue forms #83

Closed gaiborjosue closed 1 year ago

gaiborjosue commented 1 year ago

Hello,

This pull request will close #76 close #80 close #81 and close #64

This enhancement or new feature uses GitHub issue forms to enable users to submit a new model more easily and automatically. Check how the templates look here: https://github.com/gaiborjosue/trained-models-fork/issues/new/choose

You can find the workflow as .github/workflows/new_model.yml

This means that the new workflow will provide enhancement for:

  1. Organization. We are responsible for the repository structure since we ask the users for specific information. The user only has to worry about providing URLs
  2. Standardize. Since we will test the main Python script, we ensure all models being added follow a certain guideline or distribution of flags.
  3. Testing. This workflow tests Python scripts with EC2, singularity, and docker. This means we ensure their Python scripts work before adding them to the zoo.
  4. User friendly. The workflow counts with a notification/tag system that will let the user know if their request failed or had success. Also, it will automatically create a new branch for the issue and link a draft PR. So that when it is ready and has passed the checks, users can open the PR.

Next, I will attach a flow chart describing a synthesized version of the workflow since it has +500 lines: flow_addmodel

As described above, the user will create an issue form. The action will run. If it fails, it will tag the issue as failed, and the user will have to fix issues and then change the label to "Ready-to-test" for the workflow to trigger and test again.

During these processes, the workflow also has metric tracking and useful performance insights that users may check during the workflow execution in the specific Job Summary. i.e. image

@satra This new feature is 100% tested with a testing model, 'DeepCSR.'

However, a few steps are missing before this PR is/should be merged. We are working on these and will finish them as fast as possible. Perhaps @hvgazula can provide more insights into some of the following steps:

As mentioned, the "Add a new model" feature is 100% ready :). There are just a few steps before this PR can be merged. Edit: I forgot to attach the testing run of the workflow which succeeded: https://github.com/gaiborjosue/trained-models-fork/actions/runs/6442779018

hvgazula commented 1 year ago

Impressive. Thank you very much for all your efforts @gaiborjosue 👍. Now, let's wait for @satra's feedback.

satra commented 1 year ago

i'm fine with implementing these. however, i would urge to create an MVP that adds models even if they fail for various compatibility issues. i.e. get models that work in and tell users about models that don't work. that's all that is needed. leave compatibility to the user, just tell them what environment things are running in.

hvgazula commented 1 year ago

I have two models that are ready to be pushed to the repo. Will give it a try next week. @gaiborjosue Let's schedule a time next week to do one.

gaiborjosue commented 1 year ago

@hvgazula of course👍

hvgazula commented 1 year ago

@gaiborjosue If you don't mind, can you be a little more descriptive with your commit messages (if possible)? I mean you already are, but just the testing, testing2, and fixed_update weren't very informative.

gaiborjosue commented 1 year ago

Hello @hvgazula, yes. Sorry about that. Since some of them are very small changes requiring a commit to sync with the workflows, I do small messages. I will dedicate a few more seconds to better commits. Thank you for the feedback.

gaiborjosue commented 1 year ago

Hello @hvgazula, the docs are updated, and workflows are tested.

General changes since the last update:

  1. Added two new workflows to assign the user to the repo (since I realized the user can't auto-assign) and auto-tag the issue when commented "Ready-to-test" (since the user can't assign label).
  2. Allowed multiple weights/sample datasets to be uploaded via zip files.
  3. Tested PialNN successfully.

Thanks, Edward

gaiborjosue commented 1 year ago

Hello @hvgazula, I added comments on file id extraction 👍

gaiborjosue commented 1 year ago

Hello @hvgazula, nice. Now we need to add the env variables and secrets. This should be related to Ec2 and a github token with issue and workflow permissions.

hvgazula commented 1 year ago

Please email me all the relevant information. Thanks.