Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| The index of the token in the last column of the last completed row. | ||
| """ | ||
| # We get its index from the number of completed rows, i.e. the index of the first active row | ||
| return self.active_rows[0] * self.num_cols - 1 |
There was a problem hiding this comment.
Bug: Incorrect proxy token selection for new rows
The get_proxy_token_idx_for_new_row method returns the last token of the last completed row, but when starting a new row N, the proxy token should come from the previous row N-1 (which is still being generated), not from the last completed row. The formula active_rows[0] * num_cols - 1 gives the wrong row when multiple rows are active. For example, when starting row 2 with active_rows = [1, 2], it returns the last token of row 0 instead of row 1, violating the spatial adjacency principle described in the ZipAR paper.
| attentions=decoder_attentions, # type: ignore | ||
| hidden_states=decoder_hidden_states, # type: ignore | ||
| past_key_values=outputs.past_key_values, | ||
| ) |
There was a problem hiding this comment.
Bug: Wrong scores returned in generation output
The GenerateDecoderOnlyOutput returns scores=scores and logits=raw_logits, but the code builds raw_scores at line 651 which should be used instead of scores. When output_scores is enabled, raw_scores is constructed as a tuple containing the final scores, but the return statement uses the unwrapped scores variable instead. This inconsistency means the output format doesn't match the expected tuple format for scores when return_dict_in_generate and output_scores are both enabled.
|
This PR has been inactive for 10 days and is now marked as stale. |
minettekaum
force-pushed
the
feat/decoder-algorithm
branch
from
494aa08 to
d4b915c
Compare
gsprochette
left a comment
gsprochette
left a comment
There was a problem hiding this comment.
Thanks a lot, I left a single comment to indicate to the user which transformer version this is compatible with :)
| Generate the image tokens using the ZipAR algorithm. | ||
|
|
||
| This implementation was inspired by the original implementation of ZipAR: | ||
| https://github.com/ThisisBillhe/ZipAR/blob/2a5695ca2525872ac6ee38c9d62be38f0c9e985b/LlamaGen-ZipAR/autoregressive/models/generate_zipar.py. |
There was a problem hiding this comment.
Because of the instability of Janus code, this generate implementation had to be based on one specific version of transformers, namely 4.54.0 .
I think this would deserve adding a "zipar" extra to pin the transformer version if the user wants to use this algorithm.
- adding a zipar extra in the pyproject.toml
- adding a dynamic transformers version check in the
model_check_fnfunction
Ideally we would test different versions of transformer to see when this breaks, but for a first integration I would say pinning is fine and we can test & fix in a following PR
gsprochette
changed the title
minettekaum
force-pushed
the
feat/decoder-algorithm
branch
from
911b8df to
33c7bdd
Compare
minettekaum
force-pushed
the
feat/decoder-algorithm
branch
from
33c7bdd to
492dfeb
Compare
|
This PR has been inactive for 10 days and is now marked as stale. |
|
This PR has been inactive for 10 days and is now marked as stale. |
gsprochette
left a comment
gsprochette
left a comment
There was a problem hiding this comment.
LGTM, I hope we can generalize this for other transformers versions in a future PR :)
minettekaum
force-pushed
the
feat/decoder-algorithm
branch
2 times, most recently
from
5968f57 to
38d2b92
Compare
minettekaum
force-pushed
the
feat/decoder-algorithm
branch
from
7489357 to
434e2a5
Compare
2 participants
Description
The ZipAr algorithm was added to Pruna.
Type of Change
Checklist
Additional Notes
There aren't tests implemented for this algorithm yet. But it has been tested that the script runs.