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

Pushing update for metacat training #18

Merged
merged 2 commits into from
Feb 17, 2025
Merged

Conversation

shubham-s-agarwal
Copy link
Collaborator

Revamped the training for both basic and advanced notebooks with focus on simplifying the training, especially for 2-phase learning.

Revamped the training for both basic and advanced notebooks.
@mart-r
Copy link
Collaborator

mart-r commented Feb 14, 2025

The changes to meta_annotation_training.ipynb look good. Just simplifying things since you can load both LSTM and BERT models using the API rather than doing the different steps manually. Really good change IMHO.

With respect to meta_annotation_training_advanced.ipynb:
There's clearly some simplification just like in the other file. And that's great!
And there's some simplification when you get to the 2-phase learning as well. Though perhaps - to keep in line with the previous version, you could set the per-phase class weights in this part? Instead of providing the empty lists, you could use what was used before (and is provided in the demo after).
And then I'm not sure about the demo part. Looks like that's new (i.e in addition to what was included before). What value does the demo provide? It seems like the code at the end of the previous section already performs the 2-phase training of the defined meta cats. And the "demo" code wouldn't even really run since the method signatures aren't being followed. Yet there's output to it, which would suggest that it could be run, even though the disclaimer above says not to. I don't know, just feels a little confusing.

@shubham-s-agarwal
Copy link
Collaborator Author

I have removed the demo cells to make it easier to follow

Copy link
Collaborator

@mart-r mart-r left a comment

Choose a reason for hiding this comment

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

Looks good to me

@shubham-s-agarwal shubham-s-agarwal merged commit ad5ebac into main Feb 17, 2025
4 checks passed
@shubham-s-agarwal shubham-s-agarwal deleted the metacat_update branch February 17, 2025 13:54
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.

2 participants