Update to latest version of transformers #31
Conversation
|
One suggestion relevant to this PR (I'd be happy to create an issue for this if it would help): Update Same suggestion for You folks are awesome, my team is attempting to use this branch/PR for our graduate degree capstone project, we're very appreciative of your work. Thank you! |
|
Hey @jminnion, thank you for pointing out the inconsistency there! I've fixed it. Thank you for using our library :) We plan on merging this branch to main and publishing a new release soon. |
There was a problem hiding this comment.
@akashsaravanan-georgian amazing work, please fix comments before merge
| @@ -14,8 +14,8 @@ | |||
| "num_train_epochs": 5, | |||
| "overwrite_output_dir": true, | |||
| "learning_rate": 3e-3, | |||
| "per_device_train_batch_size": 12, | |||
There was a problem hiding this comment.
This was changed during a pass to standardize the training config formats across the different datasets. I modified the batch size from 12 -> 16 just to conform to the traditional approach of having the batch size be a power of 2.
| @@ -1,24 +1,25 @@ | |||
| { | |||
| "output_dir": "./logs_petfinder/", | |||
| "output_dir": "./logs_petfinder/gating_on_cat_and_num_feats_then_sum_full_model", | |||
There was a problem hiding this comment.
hm, why gating_on_cat_and_num_feats_then_sum_full_model ?
There was a problem hiding this comment.
The existing config utilized this. The name is just a reference to the method used to combine the different embeddings obtained from the model. I just standardized the format in which the output dir was named across the three configs.
| "tokenizer_name": "bert-base-uncased", | ||
| "per_device_train_batch_size": 12, | ||
| "use_simple_classifier": false, | ||
| "logging_dir": "./logs_clothing_review/bertbase_gating_on_cat_and_num_feats_then_sum_full_model_lr_3e-3/", |
There was a problem hiding this comment.
is it possible to avoid hardcoded paths like this?
There was a problem hiding this comment.
What would you suggest instead? These seem to be primarily intended to serve as examples for users more than anything else.
| CONFIGS = [ | ||
| "./tests/test_airbnb.json", | ||
| "./tests/test_clothing.json", | ||
| "./tests/test_petfinder.json" | ||
| ] | ||
|
|
||
| MODELS = [ | ||
| "albert-base-v2", | ||
| "bert-base-multilingual-uncased", | ||
| "distilbert-base-uncased", | ||
| "roberta-base", | ||
| "xlm-mlm-100-1280", | ||
| "xlm-roberta-base", | ||
| "xlnet-base-cased" | ||
| ] |
There was a problem hiding this comment.
try to use pytest fixture for this: https://docs.pytest.org/en/6.2.x/fixture.html
There was a problem hiding this comment.
I'm not sure if a fixture is the best option here. We have only one test function we're using. I was under the impression that we'd use a fixture when we have multiple tests that require similar inputs not when we have one test with several different inputs.
There was a problem hiding this comment.
Thank you for updating this repo, @akashsaravanan-georgian !! I just tried running the tests on mac and debian. Both work well with Python 3.10 💯
Includes fixes to #9, #14, #29. Also potentially resolves #3, #7, #27, #28.