-
Notifications
You must be signed in to change notification settings - Fork 399
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
Added a fix for mismatched logit and mask tensor shapes #236
Added a fix for mismatched logit and mask tensor shapes #236
Conversation
outlines/text/generate/regex.py
Outdated
@@ -129,9 +129,10 @@ def create_proposal( | |||
self.pstates = new_pstates | |||
|
|||
masks = [] | |||
mask_shape = (len(logits[0]),) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is going to work, I'd like to have an explanation as to why the vocab size does not correspond to the logit size for these models before merging. Do you have an idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I can't say for sure on how vocabs are created in pythia models, may be due to how long these models are being trained or these vocab's are shared between different models who are trained for large set of tokens, I just found that we creating a mask for logits with vocab size and since vocab size's are different at each level incase of pythia/GTPNeoXTokenizer
I thought of creating a mask directly with logit size,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on the EleuterAI discord release channel "the embedding dim is rounded up to the nearest multiple of 128 * model-parallel-size for training efficiency" in response to this question. the higher token ids are there but don't mean anything. I think this means that @xaviruvpadhiyar98's approach is correct; rounding 50254 up to nearest multiple of 128 gives you 50304.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @maedoc for confirming..
9b1d966
to
89162b6
Compare
89162b6
to
4f82db0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rlouf, is there a good test for this that can be run in CI?
@xaviruvpadhiyar98 thank you for contributing. Could you add a test in |
@rlouf I have created a test case utilizing "hf-internal-testing/tiny-random-GPTNeoXForQuestionAnswering" model def test_transformers_logits_vocab_size():
rng = torch.Generator()
rng.manual_seed(0)
model_name = "hf-internal-testing/tiny-random-GPTNeoXForQuestionAnswering"
model = models.transformers(model_name, device="cpu")
generator = generate.choice(model, ["True", "False"])
logits = torch.ones(len(model.tokenizer.vocabulary))
result = generator.create_proposal(torch.tensor([[]]), logits)
# logits = tensor([1., 1., 1., ..., 1., 1., 1.]) 1024 torch.Size([1024])
# result = tensor([[-inf, -inf, -inf, ..., -inf, -inf, -inf]]) 1024 torch.Size([1, 1024])
assert logits.shape[0] == result.shape[1] Anyways ran the test cases and pytest -vvv -W ignore::DeprecationWarning coz it was throwing some - "pkg_resources is deprecated as an API." I also had to update the conda yml file, which I have not commited # To use:
#
# $ conda env create -f environment.yml # `mamba` works too for this command
# $ conda activate outlines-dev
#
name: outlines-dev
channels:
- conda-forge
- huggingface
- pytorch
- nvidia
- xformers
dependencies:
- python==3.10.0
- jinja2
- numpy
- pillow
- pydantic
- scipy
- diffusers
- pytest
- pre-commit
- transformers
- pip
- cuda
- pytorch
- torchvision
- torchaudio
- xformers
# - pip:
# - -e ".[test]" I had issues with creating an environment with the default packages, I have added following package and installed everything with micromamba and it works flawlessly. micromamba env create -f environment.yml
|
e800c9e
to
d2051ef
Compare
d2051ef
to
96b4c03
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased and updated the test so that it simulates the logits and vocab size mismatch.
Findings with pythia-70m-deduped
Vocab size is 50304 (mentioned in original config) in https://huggingface.co/EleutherAI/pythia-70m-deduped/blob/main/config.json
Vocab size is 50254 (when we run - tokenizer.vocab_size from AutoTokenizer)
Vocab size is 50277 (when we run - len(tokenizer.get_vocab()) from AutoTokenizer)
Logit size is 50304 (when we call self.model(...) )
Now for GPT2 Vocab size and Logit size both are same 50257
This fix would create mask with size of Logit rather than vocab