Add support for training trackastra with SAM2 features#61
Conversation
|
Poof, some ruff linting structures are funny haha. All should be working now. Lemme know how it looks @C-Achard |
There was a problem hiding this comment.
Thanks @anwai98, had a quick look and the approach seems reasonable, if you end up requiring changes on the pretrained_feats repo happy to have a look as well.
One thing I noticed is that in train.py, if no model path is given (training from scratch), it will load the basic model from Trackastra, rather than the one from pretrained_feats, since in the inference-only version create() is called only from TrackingTransformer.from_folder and then it would likely crash due to the extra args.
Now you did mention you wanted to fine-tune only but maybe the best is to add some error handling if anyone tries to train a pretrained_feats model from scratch, since the resulting exception will likely look unclear if no guard is added.
Otherwise, I noticed some slightly misleading help strings in the CLI, perhaps have a look at the manuscript for better context on what these options do (I added comments on these with recommended defaults).
Finally, if your next step is to train a model, those previous configs may come in handy for that.
I hope this helps, I'm afraid I cannot test this extensively right now but happy to help further if anything is unclear in the review.
Best,
Cyril
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
|
Hi @C-Achard, Thank you so much for the detailed feedback. I'll check them out later in the evening and come back to you! |
….com/anwai98/trackastra into add-training-support-with-sam2-feats
|
Hi @C-Achard, Sorry for the super late follow-up. I managed to come back to the PR this week only - had a couple of busy weeks in the past. I took care of the comments you left. What do you think about the current state now? |
| FeatureExtractor, | ||
| WRPretrainedFeatures, | ||
| ) | ||
| if self.pretrained_n_augs != 3: |
There was a problem hiding this comment.
Does it actually generate the augmented copies ? That would be important for performance, but that might not be available on the pretrained_feats repo API.
If you're seeing too much overfitting that would be the first suspect, I can port the augmentation part to pretrained_feats if needed, lmk @anwai98
There was a problem hiding this comment.
Hi @C-Achard,
Your hunch is right, the augmented copies are not being generated in the current state (and I raise a warning in data.py that shows pretrained_n_augs is not yet wired into FeatureExtractor).
Let's do it this way (if you agree): I'll run a quick training and see how the model performs, compared to the general SAM2 model. If I see overfitting, I'll take you up on the offer to port the augmentation API. Thanks! ;)
Nice, thanks! This looks good as far as I can tell, definitely curious to see how this performs on your data. If it overfits too much I would look into the augmentation API (I can help) but this is likely not needed for a first check. Let me know if I can help with anything else, thanks again |
Hi @C-Achard,
Here's are my minimal changes to make training work with SAM2 features.
Let me know how it looks!
PS. In case it helps, here's my yaml config file to train
trackastra:yaml config