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

[Big Update] Implement separate callbacks class #21

Merged
merged 26 commits into from
Mar 15, 2022
Merged

[Big Update] Implement separate callbacks class #21

merged 26 commits into from
Mar 15, 2022

Conversation

kaylode
Copy link
Owner

@kaylode kaylode commented Mar 14, 2022

Helps Trainer more generalized and easy to be modified/intergrated

@kaylode kaylode added the enhancement New feature or request label Mar 14, 2022
@kaylode kaylode self-assigned this Mar 14, 2022
@kaylode
Copy link
Owner Author

kaylode commented Mar 14, 2022

Hi @lannguyen0910 , welcome to the project. Can you help me test this branch? See whether the callbacks are simple to use and customize then give me some feedbacks/suggestion.

@kaylode
Copy link
Owner Author

kaylode commented Mar 14, 2022

There is a CALLBACK_REGISTRY in which contains all callbacks function that are registered in pipeline.yaml file. All new callback classes must be defined with the same method names as in theseus/base/callbacks/base_callbacks.py. Then these methods will be automatically called by the trainer while training.

@lannguyen0910
Copy link
Collaborator

lannguyen0910 commented Mar 15, 2022

@kaylode I have tested on the classification + segmentation on this branch.
In classification:

  • the visualizer on tensorboard works fine.
  • no warnings or errors triggered.
  • the logger also works fine.
  • The loading, training is fast, and good.
  • The model works ok, although it reported more error cases afterward, you can check the SupervisedTrainer again to see whether it has any potential error, for me it looks fine.
  • Besides, it also works good without the visualizer callback.

In segmentation:

  • Apart from the callbacks, i think we have an error with segmentation_models.pytorch dependency on create_model method, i suggest using git+https://github.com/qubvel/segmentation_models.pytorch instead.
  • I think we have an error in _load_mask func in CSVDataset, it reports RuntimeError: Class values must be smaller than num_classes. in one-hot encoding the masks. So i try the previous implement to see if it pass-through the error.

image

I think my attempt is wrong because the sanity check plot this.

image

Overall, the callbacks works good, no major problems, i will take a deep look in the code again to see if there is minor bugs or improvements, i will be testing on WandB too.

@kaylode
Copy link
Owner Author

kaylode commented Mar 15, 2022

@lannguyen0910

Apart from the callbacks, i think we have an error with segmentation_models.pytorch dependency on create_model method, i suggest using git+https://github.com/qubvel/segmentation_models.pytorch instead.

Yes, I've been experiencing this as well. Basically it is due to the version of timm models. I think your suggestion is fine, do you know how can we include that in setup.py ?

I think we have an error in _load_mask func in CSVDataset, it reports RuntimeError: Class values must be smaller than num_classes. in one-hot encoding the masks. So i try the previous implement to see if it pass-through the error.

Yes, this happened because we must define a different _load_mask() function for the specific dataset (CarvanaCar in this case). The default _load_mask() in the current settings works for masks in which each pixel present a class.

Thank you very much, I'm gonna merge this branch and if there are any more bugs we will fix that later.

p/s: if you want to comment on any parts of the template that you see inconvenient or unnecessary, or want to contribute improvement, just create a new branch and make a PR. :>

@kaylode kaylode merged commit 35e024a into master Mar 15, 2022
@kaylode kaylode deleted the callbacks branch March 15, 2022 13:53
@kaylode
Copy link
Owner Author

kaylode commented Mar 15, 2022

Solves #19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants