test: Added a slight distortion in bazel - //test/orfs/mock-array:MockArray_8x8_flat_cts_openroad_test

[openroad-ci]

Fixes #9062

Problem

Bazel //test/orfs/mock-array:MockArray_8x8_flat_cts_openroad_test suffers from intermittent failure.

Root-cause

1. Cause of intermittent failure

Non-determinism (ND) described and resolved in parallaxsw/OpenSTA#359

• Pointer-based ordered map caused the ND issue.
• Once the PR is merged and OR uses the latest STA, this intermittent failure will be resolved.

2. Inappropriate test case setup in test/orfs/mock-array/power.tcl

Test case sequence

1. Report power based on VCD dump (total_power_1)
2. Dump the computed activity in an user activity file (Same value as the VCD flow)
3. Read the user activity file and report power again based on the user activity (total_power_2)
4. Checker 1: total_power_1 != total_power_2

• If the user activity loading fails, the switching activity values based on VCD flow will not be overwritten by the user activity annotation and total_power_1 == total_power_2. Checker 1 is intended to catch such failure.

5. Checker 2: abs(total_power_1 - total_power_2) < 1e-3

• If there is a large error, something is wrong in the user activity flow.

So the test case expects a slight error b/w total_power_1 and total_power_2.
But this is not a robust test because the two total_power numbers are basically computed from the same set of float numbers.
The slight difference comes from the float-accumulation error because the float-accumulation sequences are different b/w VCD flow and user-activity flow.

Because of the ND-issue,

• Sometimes (~70%), total_power_1 and total_power_2 are slightly different --> TEST PASS
• Otherwise (~30%), total_power_1 and total_power_2 are exactly the same --> TEST FAIL

Solution

Once parallaxsw/OpenSTA#359 is merged, the intermittent failure will be gone.

But more robust test is to modify the test case in order to produce slightly different power numbers b/w VCD and user-activity flows.

If we intentionally distort a few of the activity factors (for the first three pins) in user-activity file, VCD and user-activity flows will use different sets of float numbers to accumulate and total_power_1 != total_power_2 will be satisfied always.
This is more robust test and the test case will not suffer from the ND issue.

[gemini-code-assist]

Code Review

This pull request addresses an intermittent test failure by introducing a deliberate distortion in the test data. The change modifies the power.tcl test script to alter the duty cycle for a few pins, ensuring a predictable difference in power calculations and making the test more robust. My review identified a minor discrepancy between the intended logic (modifying three pins) and the implementation (modifying four pins), and I've provided a suggestion to correct it.

[gemini-code-assist]

The condition $vcd_pin_count <= 3 will modify the duty cycle for the first four pins (when vcd_pin_count is 0, 1, 2, and 3). However, the comment on line 38 and the pull request description state that only the first three pins should be modified. To align with the intended logic, the condition should be changed to $vcd_pin_count < 3.

  if { $vcd_pin_count < 3 } {

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[jhkim-pii]

Once user activity file is read, the total power is recomputed and std::map<const Instance*, PowerResult> instance_powers_; is filled with different PowerResult objects w/ different pointer addresses (but the same float numbers).
--> float accumulation sequence changes
--> Slightly different total power for each run (ND issue).

My fix will produce different PowerResult numbers b/w VCD and user activity flows. So the test will pass always regardless of the ND issue.

[jhkim-pii]

FWIW,

Before the ND issue fix (by parallaxsw/OpenSTA#359)

• Total power numbers change in each run.

#1
Total power from VCD: 0.23302288353443146
Total power from user activity: 0.23302286863327026

#2
Total power from VCD: 0.23302291333675385
Total power from user activity: 0.23302288353443146

#3
Total power from VCD: 0.23302264511585236
Total power from user activity: 0.23302267491817474

After the ND issue fix

• The same total power numbers in each run.

#1
Total power from VCD: 0.23309363424777985
Total power from user activity: 0.2330935150384903

#2
Total power from VCD: 0.23309363424777985
Total power from user activity: 0.2330935150384903

#3
Total power from VCD: 0.23309363424777985
Total power from user activity: 0.2330935150384903

[jhkim-pii]

@oharboe FYI

[oharboe]

Thanks for curating and coming up with arobust plan for this use case.

[maliberty]

@oharboe I thought the goal of this check was to compare sta computed activities to vcd ones not vcd to itself. Is that intentional?

[oharboe]

The goal is to run through the flow to check that all.pins are annotated simulation works, etc. A demonstration and basic smoke test of the use case.

I intended to check that default pin activations power and vcd power was different.

If I get around to it, I would switch to generating .saif and using read_saif

[maliberty]

@jhkim-pii could you change the test to "I intended to check that default pin activations power and vcd power was different." rather than " distort a few of the activity factors "

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[jhkim-pii]

If we intentionally distort a few of the activity factors (for the first three pins) in user-activity file, VCD and user-activity flows will use different sets of float numbers to accumulate and total_power_1 != total_power_2 will be satisfied always.

@jhkim-pii could you change the test to "I intended to check that default pin activations power and vcd power was different." rather than " distort a few of the activity factors "

Unfortunately, I cannot edit the PR description above because the author is openroad-ci. If you have the edit permission, you can do it.

I updated the comment in test/orfs/mock-array/power.tcl.

[maliberty]

I don't see a change from distortion to the intended approach.

[jhkim-pii]

Oh, I misunderstood before.

I reduced the duty and activity values by a factor or 10, and added read back checks for the annotated activity values.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[maliberty]

I mean if you don't read a vcd then sta will estimate activities and the corresponding power. That should be the first value. Then read the vcd and compute the power as the second value. Those should never match without any trickery.

[oharboe]

I mean if you don't read a vcd then sta will estimate activities and the corresponding power. That should be the first value. Then read the vcd and compute the power as the second value. Those should never match without any trickery.

Yes, this was my plan with this test

[jhkim-pii]

• Removed the tricky 10x activity scale down.
• Reported the estimated power before reading VCD.
• Refactored power.tcl for better readability.
• Changed file name from load_power.tcl to load_mock_array.tcl.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[jhkim-pii]

• Removed the read_vcd command.
• load_power.tcl name does not look good. So renamed it, and the renaming caused more changes in other files.