Skip to content

[codex] stabilize trelu golden dir mapping#56

Open
zhoubot wants to merge 1 commit intomainfrom
codex/fix-trelu-golden-dir-mapping
Open

[codex] stabilize trelu golden dir mapping#56
zhoubot wants to merge 1 commit intomainfrom
codex/fix-trelu-golden-dir-mapping

Conversation

@zhoubot
Copy link
Copy Markdown
Collaborator

@zhoubot zhoubot commented Apr 8, 2026

What changed

  • replace the index-based i < 8 directory selection in trelu/gen_data.py with explicit per-case output names
  • map the BF16 case through the same explicit mechanism instead of relying on loop position

Why

The previous logic depended on the current ordering and size of case_params_list. Any insertion, deletion, or reordering could silently break the mapping between generated golden-data directories and the gtest names used by TRELUTest.

Impact

  • trelu golden-data generation is now tied to the intended gtest case names instead of fragile list indexes
  • future testcase additions or reordering will not accidentally break the directory layout expected by the CPU-SIM test binary

Validation

  • python3 -m py_compile tests/cpu/st/testcase/trelu/gen_data.py
  • ruff check tests/cpu/st/testcase/trelu/gen_data.py
  • python3 tests/run_cpu.py --testcase trelu --gtest_filter 'TRELUTest.case_0' --generator Ninja --build-dir build/cpu_pr_trelu_dir_mapping --clean

Copy link
Copy Markdown

@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 refactors the TRelu test data generation script by introducing an explicit output_case_name attribute to the TReluParams class, which is now used for directory naming instead of loop indices. A review comment suggests that the case_name variable and the generate_case_name function have become redundant following these changes and should be removed to simplify the script.

output_dir = os.path.join(script_dir, f"TRELUTest.case_{i}")
else:
output_dir = os.path.join(script_dir, case_name)
output_dir = os.path.join(script_dir, f"TRELUTest.{param.output_case_name}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The case_name variable (defined on line 81) is no longer used for directory mapping, as it has been replaced by the explicit param.output_case_name. Additionally, case_name is passed to gen_golden_data_trelu (line 86), which does not use the parameter. This makes the generate_case_name function and the case_name variable redundant. Consider removing them to simplify the script and improve maintainability.

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.

1 participant