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

Bugfix/enforce model trained once #519

Merged
merged 9 commits into from
Oct 2, 2024
Merged

Conversation

yrkim98
Copy link
Collaborator

@yrkim98 yrkim98 commented Sep 27, 2024

UI changes to enforce that the user can only train a model once. Will automatically switch to prediction tab and disable curation/training once plugin is able to verify a model was sucessfully trained.

Changes:

  • src/allencell_ml_segmenter/main/main_model.py: Dispatch training complete event
  • src/allencell_ml_segmenter/main/main_widget.py: Subscribe to event and disable tabs when training complete
  • src/allencell_ml_segmenter/training/view.py: Add check to see if training completed successfully, and dispatch event if so

@yrkim98 yrkim98 force-pushed the bugfix/enforce-model-trained-once branch from 428af07 to eac82b3 Compare October 1, 2024 21:39
Copy link
Collaborator

@amilworks amilworks left a comment

Choose a reason for hiding this comment

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

Just so I understand correctly, this is to ensure that after someone has trained a model through the plugin, they do not mistakenly retrain again. So this disables the tab for training and puts them directly to the prediction tab?

How does this work for iterative training? Say I trained my model, I run prediction, results look good. Now I want to retrain again on new data, but load the model I just trained and continue training from there, do I need to close the Segmenter ml plugin since training is disabled? Or is it easy to just start a new run? Sorry for the weird phrasing, I hope this makes sense.

@hughes036
Copy link
Collaborator

How does this work for iterative training? Say I trained my model, I run prediction, results look good. Now I want to retrain again on new data, but load the model I just trained and continue training from there, do I need to close the Segmenter ml plugin since training is disabled? Or is it easy to just start a new run? Sorry for the weird

Good questions. I think it is to be in line with the existing workflow for newly trained models (which can't be iteratively trained, per the UX design). Allowing iterative training on the same model was going to open a can of worms in terms of populating the UI correctly and validating what is entered to be coherent with the earlier rounds of training (I think this was the reason - can you confirm @yrkim98 @thao-do )

The workflow that we ended up with (initially during a conversation with Susan) was that for every iteration of training, the user will create a new model based on the weights that they are 'iterating' on. So in your scenario, rather than doing additional training on model 1 after a prediction run, they create model 2 (a new model) using the weights from model 1 and do a single round of training on that.

Did I get it right @yrkim98 ?

@hughes036
Copy link
Collaborator

@yrkim98 @amilworks How about we discuss this @ standup?

@thao-do
Copy link
Collaborator

thao-do commented Oct 2, 2024

@hughes036 I think we'll still have to deal with that can of worm to a certain extent - we need to check with @benjijamorris if any UI input will be in conflict with the model weight being used (mainly the Patch size).

Also, the Training tab was currently disabled in the existing model route since we couldn't quite get iterative training earlier on (I need to look back on the reasons why to see whether they're still a concern). This new addition to pull model weight is a first step toward allowing more than one case of iterative training. We'll need more discussion on this as there are a few things we might want to reassess.

@yrkim98 yrkim98 merged commit 59a4573 into main Oct 2, 2024
4 checks passed
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.

4 participants