Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
update utils and examples #586
Conversation
|
Check out this pull request on Review Jupyter notebook visual diffs & provide feedback on notebooks. Powered by ReviewNB |
examples/named_entity_recognition/ner_wikigold_transformer.ipynb
Outdated
Show resolved
Hide resolved
examples/named_entity_recognition/ner_wikigold_transformer.ipynb
Outdated
Show resolved
Hide resolved
| @@ -77,7 +87,7 @@ def get_inputs(batch, device, model_name, train_mode=True): | |||
| Labels are only returned when train_mode is True. | |||
| """ | |||
| batch = tuple(t.to(device) for t in batch) | |||
| if model_name.split("-")[0] in ["bert", "distilbert"]: | |||
| if model_name in list(TC_MODEL_CLASS): | |||
daden-ms
May 6, 2020
Collaborator
model_name is not optional in this function. The documentation needs to be changed.
model_name is not optional in this function. The documentation needs to be changed.
daden-ms
May 6, 2020
Collaborator
can do_lower_case be handled within the function, instead of being passed as an argument to avoid mismatch?
can do_lower_case be handled within the function, instead of being passed as an argument to avoid mismatch?
| "Text after tokenization with length {} has been truncated".format( | ||
| len(new_tokens) | ||
| ) | ||
| ) | ||
| new_tokens = new_tokens[:max_len] | ||
| new_labels = new_labels[:max_len] |
daden-ms
May 6, 2020
Collaborator
function fit_to_block_size and build_mask from abstractive_summarization_bertsum.py (originally from huggingface's transformers) can be reused here and probably much easier to comprehend.
function fit_to_block_size and build_mask from abstractive_summarization_bertsum.py (originally from huggingface's transformers) can be reused here and probably much easier to comprehend.
daden-ms
May 6, 2020
Collaborator
I feel processing the dataset per example is much cleaner way to see what preprocessing is needed. Also if you choose to process the dataset one example at a time, there is no need to return a TensorDataset, a list of dictionary suffices, and in your get_inputs function, you can specify the field, instead of using the index 0, 1 to track what the inputs requires. It's up to you to decide whether you want to make this change or not.
I feel processing the dataset per example is much cleaner way to see what preprocessing is needed. Also if you choose to process the dataset one example at a time, there is no need to return a TensorDataset, a list of dictionary suffices, and in your get_inputs function, you can specify the field, instead of using the index 0, 1 to track what the inputs requires. It's up to you to decide whether you want to make this change or not.
| @@ -398,7 +405,9 @@ def fit( | |||
|
|
|||
| # init scheduler | |||
| scheduler = Transformer.get_default_scheduler( | |||
| optimizer=self.optimizer, warmup_steps=warmup_steps, num_training_steps=max_steps | |||
| optimizer=self.optimizer, | |||
daden-ms
May 6, 2020
Collaborator
should we use dataset instead of dataloader to make it consistent with other transformer models?
should we use dataset instead of dataloader to make it consistent with other transformer models?
daden-ms
May 6, 2020
•
Collaborator
same for predict function
same for predict function
daden-ms
May 6, 2020
Collaborator
can the predict function take the label_map to provide the entity labels directly without having the users call the get_predicted_token_labels?
can the predict function take the label_map to provide the entity labels directly without having the users call the get_predicted_token_labels?
|
Nice work. I put in some comments and might require significant changes. It's up to you to decide whether you want to make those changes or not, The PR looks great by itself. |
|
looks good to me. Just one last comment, do we want to check-in the output of the cells in the Jupiter notebooks? |
In general, yes. The NER notebook now includes cells' output. |
…ecipes into bleik/add-models
Description
Checklist: