fix: resolve TypeError when max_tokens=None in generate() and CLI#296
Open
voidborne-d wants to merge 1 commit intonari-labs:mainfrom
Open
fix: resolve TypeError when max_tokens=None in generate() and CLI#296voidborne-d wants to merge 1 commit intonari-labs:mainfrom
voidborne-d wants to merge 1 commit intonari-labs:mainfrom
Conversation
Fixes nari-labs#281. Root cause: when CLI omits --max-tokens (default None), it passes max_tokens=None to Dia.generate(), which overrides the hardcoded default of 3072. The generation loop then compares dec_step < None, raising TypeError: '<' not supported between instances of 'int' and 'NoneType'. Changes: - dia/model.py: change generate() signature from max_tokens: int = 3072 to max_tokens: int | None = None; resolve None to config.decoder_config.max_position_embeddings at function entry, before any comparison. This is consistent with how _prepare_generation already handles None via its own or-fallback. - cli.py: only pass max_tokens when user explicitly sets --max-tokens; import and use DEFAULT_SAMPLE_RATE from dia.model instead of hardcoded 44100 to keep sample rate in sync with the model constant. - tests/test_generate_max_tokens.py: 15 regression tests covering signature, None resolution order, CLI integration, constant consistency, and exact reproduction of the issue nari-labs#281 error path. Tests parse source code directly to avoid requiring torch/torchaudio in the test environment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #281.
When
cli.pyomits--max-tokens(defaultNone), it passesmax_tokens=NonetoDia.generate(), which overrides the hardcoded default of3072. The generation loop then comparesdec_step < None, raising:Root Cause
generate()declaredmax_tokens: int = 3072, but callers (includingcli.py) pass explicitNonewhich overrides the default. Two comparisons depend onmax_tokensbeing an integer:Note that
_prepare_generation()already handlesNonecorrectly viamax_generation_length or config.decoder_config.max_position_embeddings, so only thegenerate()function itself crashes.Changes
dia/model.pygenerate()signature:max_tokens: int = 3072→max_tokens: int | None = Noneif max_tokens is None: max_tokens = self.config.decoder_config.max_position_embeddings_prepare_generationalready handlesNoneintvalues are unaffectedcli.pymax_tokensingenerate()kwargs when user explicitly sets--max-tokens, avoiding the None override entirelyDEFAULT_SAMPLE_RATEfromdia.modelinstead of hardcoded44100to keep the sample rate in sync with the model constanttests/test_generate_max_tokens.pyNone, default isNoneNone, guards against passingNone, importsDEFAULT_SAMPLE_RATE, no hardcoded44100NonedefaultDEFAULT_SAMPLE_RATEdefined,save_audio()uses itint < NoneraisesTypeError, verifies no unguarded comparisonsTesting