Skip to content

Conversation

luarss
Copy link
Contributor

@luarss luarss commented Apr 24, 2025

This pull request updates the BaseAlgoEvalSmokeTest class in tools/AutoTuner/test/smoke_test_algo_eval.py to introduce the concept of a flow_variant for better configurability and reuse. The changes primarily focus on refactoring the setUp method and related commands to use this new variable.

Refactoring for flow_variant integration:

  • Introduced a new class variable flow_variant with the default value "smoke-test-algo-eval" to represent the flow variant. ([tools/AutoTuner/test/smoke_test_algo_eval.pyR47-R55](https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/pull/3108/files#diff-63a772b287b174f31b4335be42c7e910f197140112906da7b1bb169517dfacf8R47-R55))
  • Updated the setUp method to use flow_variant in constructing the experiment name and the reference file path, improving flexibility and consistency. ([tools/AutoTuner/test/smoke_test_algo_eval.pyR47-R55](https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/pull/3108/files#diff-63a772b287b174f31b4335be42c7e910f197140112906da7b1bb169517dfacf8R47-R55))
  • Refactored the make_base method to include FLOW_VARIANT in all make commands, ensuring that the flow variant is explicitly passed to the build system. ([tools/AutoTuner/test/smoke_test_algo_eval.pyL71-R79](https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/pull/3108/files#diff-63a772b287b174f31b4335be42c7e910f197140112906da7b1bb169517dfacf8L71-R79))

@luarss luarss added the autotuner Flow autotuner label Apr 24, 2025
@luarss
Copy link
Contributor Author

luarss commented Apr 25, 2025

Pending fix: #3025

@vvbandeira
Copy link
Member

The tests should be independent and self-contained. If we need the file, it should be generated by the test.

@luarss luarss force-pushed the topic/avoid-cleanall-at branch from fea45b7 to a0ead00 Compare May 1, 2025 08:13
luarss added 3 commits May 3, 2025 05:18
- because clean_all was used, the original metadata.json was nuked.

Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
@luarss luarss force-pushed the topic/avoid-cleanall-at branch from a0ead00 to 399356c Compare May 3, 2025 05:18
@luarss luarss requested a review from Copilot May 3, 2025 15:48
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a metadata.json regression in the AlgoEval tests by ensuring that the correct metadata file is used and the appropriate cleanup is performed.

  • Introduces a new class variable "flow_variant" to standardize test naming.
  • Updates the reference file name to include the flow variant.
  • Modifies make command invocations to pass the FLOW_VARIANT environment variable.
Comments suppressed due to low confidence (1)

tools/AutoTuner/test/smoke_test_algo_eval.py:80

  • Consider capturing standard output and error (e.g., using the 'capture_output=True' parameter or redirecting stdout and stderr) to aid in debugging if the subprocess command fails.
out = subprocess.run(command, shell=True)

@luarss luarss requested a review from vvbandeira May 3, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autotuner Flow autotuner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants