Skip to content

update neo++ seed#1032

Merged
llmc-reviewer merged 1 commit intomainfrom
nn
Apr 22, 2026
Merged

update neo++ seed#1032
llmc-reviewer merged 1 commit intomainfrom
nn

Conversation

@helloyongyang
Copy link
Copy Markdown
Contributor

No description provided.

@llmc-reviewer llmc-reviewer merged commit 708c30c into main Apr 22, 2026
2 checks passed
@llmc-reviewer llmc-reviewer deleted the nn branch April 22, 2026 07:39
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates example scripts with new file paths and null seeds, makes scipy imports optional in the WanMatrixGame3 runner, and adds a conditional check for seeding in the pipeline. Feedback highlights that skipping seed initialization could lead to crashes in runners expecting an integer, suggesting a random seed be generated instead. Furthermore, the use of hardcoded absolute paths in examples is discouraged for portability, and the new optional scipy imports require safety checks in the functions that utilize them to avoid runtime errors.

Comment thread lightx2v/pipeline.py
Comment on lines +463 to +464
if self.seed is not None:
seed_all(self.seed)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Setting self.seed to None and skipping seed_all will result in input_info.seed being None. This will cause a crash in runners that use torch.manual_seed(input_info.seed) (such as WanMatrixGame3Runner at line 1624). It is better to initialize self.seed with a random integer if it is None to ensure downstream compatibility.

Suggested change
if self.seed is not None:
seed_all(self.seed)
if self.seed is None:
self.seed = torch.randint(0, 2**32, (1,)).item()
seed_all(self.seed)

pipe.generate(
seed=200,
save_result_path="/data/nvme1/yongyang/FL/LightX2V/save_results/output_lightx2v_neopp_dense_2k_0.png",
save_result_path="/data/nvme1/yongyang/kkk/LightX2V/save_results/output_lightx2v_neopp_dense_2k_0.png",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding absolute paths that include personal user directories (e.g., /yongyang/kkk/) makes the example less portable and harder for others to use. Consider using relative paths or environment variables for output directories.

Comment on lines +18 to +24
try:
from scipy.interpolate import interp1d
from scipy.spatial.transform import Rotation, Slerp
except ImportError:
interp1d = None
Rotation = None
Slerp = None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Wrapping these imports in a try-except block without corresponding checks in the functions that use them (e.g., _matrix_game3_interpolate_camera_poses) will lead to a TypeError at runtime if scipy is missing. If scipy is an optional dependency, please add checks to provide a more helpful error message when these features are accessed.

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