OpenROAD icon indicating copy to clipboard operation
OpenROAD copied to clipboard

Moved DB_ECO debug log location before block->journal_ check

Open openroad-ci opened this issue 2 weeks ago • 8 comments

Changes

  1. Move DB_ECO debug logs before blcok->journal_ check to use the debug logs for ODB debugging.
image
  1. Set debug level 1 for major ODB changes (create, destroy, connect, disconnect, ...). Set debug level 2 for minor ODB changes (setSigType, setIoType, ...).
  2. _dbDatabase::default_logger_ is added to resolve test case issues.

Problem

Once the debug log is moved, the following tests fail.

The following tests FAILED:
        178 - odb.test_block.tcl (Failed)                       IntegrationTest tcl odb passfail
        180 - odb.test_bterm.tcl (Failed)                       IntegrationTest tcl odb passfail
        182 - odb.test_destroy.tcl (Failed)                     IntegrationTest tcl odb passfail
        184 - odb.test_group.tcl (Failed)                       IntegrationTest tcl odb passfail
        190 - odb.test_module.tcl (Failed)                      IntegrationTest tcl odb passfail
        192 - odb.test_net.tcl (Failed)                         IntegrationTest tcl odb passfail

For example, odb.test_bterm.tcl fails with the following ODB-0001 ERROR.

/workspace/ws9/OpenROAD-flow-scripts/tools/OpenROAD/src/odb/test $ $OR test_bterm.tcl 
OpenROAD v2.0-26742-gb3cbba7894 
Features included (+) or not (-): +GPU +GUI +Python
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.
[CRITICAL ODB-0001] No logger is installed in odb.

It is because the integration test does not create OpenRoad instance, just creates dbDatabase w/o logger creation in the simple test. So ODB-0001 occurred due to the absence of logger instance.

Solution

To avoid the test issues, I added a inline static utl::Logger default_logger_ = utl::Logger(); in _dbDatabase.h, which creates a default logger when _dbDatabase is created.

Although the default logger is redundant in the real OpenROAD use case, its memory overhead is trivial.

openroad-ci avatar Dec 08 '25 09:12 openroad-ci

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Dec 08 '25 09:12 github-actions[bot]

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Dec 08 '25 11:12 github-actions[bot]

If you look at test_inst.tcl you'll see they avoid the logger issue by using createSimpleDB 1 to use the default db that already has a logger. I prefer to not have a default logger.

maliberty avatar Dec 08 '25 17:12 maliberty

If you look at test_inst.tcl you'll see they avoid the logger issue by using createSimpleDB 1 to use the default db that already has a logger. I prefer to not have a default logger.

createSimpleDB 1 is not suitable for the unit test that contains multiple tests w/ database create & destroy because we cannot destroy and recreate the default DB.

I found that there is a default logger already (utl::Logger::defaultLogger()). So I used it instead of adding a new dbDatabase::default_logger_ instance.

jhkim-pii avatar Dec 09 '25 03:12 jhkim-pii

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Dec 09 '25 04:12 github-actions[bot]

PR-head CI (Bazel mock-array test) fails repeatedly with the following error, which is weird because this PR does not affect QoR. Moreover, PR-merge CI passed and no such issue in my local run. I think this is fine to merge. Updating the rule file just for this weird PR-head fail looks not good.

[2025-12-09T06:30:50.910Z] [ERROR] cts__timing__hold__ws fail test: -25.4265 >= -15.7
[2025-12-09T06:30:50.910Z] [ERROR] cts__timing__hold__tns fail test: -376.289 >= -93.3
...
[2025-12-09T06:30:50.910Z] Failed metadata checks: 2 out of 43
[2025-12-09T06:30:50.910Z] make: *** [/home/user/.cache/bazel/_bazel_user/ce393be8d0199af5e637d28344530229/sandbox/processwrapper-sandbox/12026/execroot/_main/bazel-out/k8-fastbuild/bin/test/orfs/mock-array/make_MockArray_8x8_flat_test_8x8_flat_test.runfiles/_main/external/bazel-orfs++orfs_repositories+docker_orfs/OpenROAD-flow-scripts/flow/util/utils.mk:23: metadata-check] Error 1

jhkim-pii avatar Dec 09 '25 06:12 jhkim-pii

Since pr-merge is passing I'm not worried about pr-head.

maliberty avatar Dec 09 '25 19:12 maliberty

Any thoughts on my two comments above?

maliberty avatar Dec 09 '25 19:12 maliberty

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Dec 12 '25 04:12 github-actions[bot]

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Dec 12 '25 04:12 github-actions[bot]

Updated the code based on your feedbacks.

jhkim-pii avatar Dec 12 '25 08:12 jhkim-pii