Repository navigation
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the build_text_encoder method in z_image_checkpointer.py to support both transformers 4.x and 5.x configuration formats by checking for rope_theta within rope_parameters as well as at the top level of the configuration dictionary. Additionally, a new test suite z_image_checkpointer_test.py has been introduced to verify correct behavior across these different configuration structures. There are no review comments, and I have no further feedback to provide.
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
ZImageCheckpointer.build_text_encoderreadsrope_thetafrom the top level of the text encoder config dict. transformers 5.x moved it intorope_parameters, so with transformers 5.x loading Z-Image from the Diffusers checkpoint fails withKeyError: 'rope_theta'. This readsrope_parameters["rope_theta"]when it is there and falls back to the top-level key, the same way the FLUX.2-klein loader already does (generate_flux2klein.py).Issue
pip install .takes its dependencies fromdependencies/requirements/generated_requirements/requirements.txt, which allowstransformers>=4.57.6, so a fresh install can resolve transformers 5.x (base_requirementssaystransformers<5.0.0). With transformers 5.20.1:load_diffusers_pipelinepasses this dict tobuild_text_encoder, which raises:The Orbax restore path calls
build_text_encoderwith the config saved in the checkpoint metadata, so restoring a cache whose metadata was written under transformers 5.x hits the same error (caches written under 4.x still restore).Changes
checkpointing/z_image_checkpointer.py: takerope_thetafromrope_parametersif present, otherwise from the top level. If neither exists it still raisesKeyError, as before.tests/z_image/z_image_checkpointer_test.py(new): builds the text encoder abstractly from a tiny Qwen3 config (no weights downloaded) and checksrope_thetafor the 4.x layout, the 5.x layout, andAutoConfig.for_model("qwen3", ...).to_dict()from whatever transformers is installed, plus that a config with norope_thetaanywhere still raisesKeyError.Testing
New test, JAX CPU:
KeyError: 'rope_theta'), 2 passedReal weights with transformers 5.20.1: I built and loaded the
Tongyi-MAI/Z-Image-Turbotext encoder throughbuild_text_encoderandload_qwen3_weightsin float32 and compared the layerencode_stepreturns (all_hidden_states[-2], index 35) withhidden_states[35]from the transformers PyTorch model in float32, on a 17-token prompt underjax.default_matmul_precision("highest"): per-token cosine 1.000000 (min and mean), relative L2 2.7e-6. On main the same script stops at theKeyError.Checks:
pyink --check --pyink-indentation=2 --line-length=125on the touched files is clean,ruff check .from the repo root passes,pylintreports no new messages (the checkpointer file has the same 5 existing messages as on main, the test file rates 10.00).