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

verbose message to increase size_embeddings_count #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

e3oroush
Copy link

@e3oroush e3oroush commented Dec 9, 2021

Hi
First, thank you for the great effort, I learned a lot from you.
I tried to use your model for my own dataset, but since the length of entity spans are a bit larger than the default size_embeddings_count in the config, I wasn't successful. I was getting this error message, which wasn't clear enough.

IndexError: index out of range in self

It took me a whole day to dig the bug up, I tried to change your code to have a more verbose message about this issue.
I hope it can help others with a similar problem.

@markus-eberts
Copy link
Member

markus-eberts commented Jan 7, 2022

Hi @e3oroush,

thank you very much for your pull request! This would indeed be a nice addition. However, I think it is better to just check the largest entity span of the input datasets (train/dev/test) and raise an exception in case it exceeds the size_embeddings_count. This could be done after the datasets were loaded. This way, the exception (or assertion) doesn't occur sometime during the training process. What do you think?

@e3oroush
Copy link
Author

e3oroush commented Jan 9, 2022

Thanks for answering.
I'm not completely sure where you are exactly mentioning. But, because it's your code base, I think your idea makes definitely more sense than my PR. 😄

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