Add features: dry-run option, lower memory usage - #13
Conversation
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds CLI- and configuration-controlled dry-run characterization, propagates sentinel values through measurement procedures, narrows simulation failure handling, updates Liberty group construction, and makes ChangesDry-run characterization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Characterizer
participant MeasurementProcedures
participant Simulator
participant Liberty
CLI->>Characterizer: pass --no-sim as dry_run
Characterizer->>MeasurementProcedures: execute characterization
MeasurementProcedures->>Simulator: run when dry_run is false
MeasurementProcedures->>Liberty: write measured or sentinel values
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charlib/characterizer/procedures/combinational/delay.py`:
- Around line 188-197: Update the delay calculation around delay_measurements so
the -1 sentinel is used only when settings.dry_run is enabled; during real runs,
propagate or raise an error when no measurements are available instead of
writing a fabricated LUT value. Preserve the existing measured-delay conversion
and LookupTable flow for non-empty measurements.
In
`@charlib/characterizer/procedures/sequential/constraint/metastability/c2q_contour.py`:
- Around line 228-234: Update the settings.dry_run branch in the sequential
metastability contour procedure to use -1 @ PySpice.Unit.u_s for both
placeholder timing values in result_per_state[state_str]. Preserve the existing
None third value and continue behavior, matching the -1 sentinel convention used
by the other dry-run stubs.
- Around line 552-556: Reorder the dry_run handling in get_t_stabilizing so
sim_latch(...) is invoked before the early return, matching get_c2q and
preserving generated debug artifacts during dry runs. Keep the existing -1 *
PySpice.Unit.u_s return value and quiet-message behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 84ba5651-074e-4a19-a020-13cb6960277e
📒 Files selected for processing (8)
charlib/characterizer/characterizer.pycharlib/characterizer/procedures/combinational/delay.pycharlib/characterizer/procedures/combinational/leakage_power.pycharlib/characterizer/procedures/pin_capacitance/ac_sweep.pycharlib/characterizer/procedures/pin_capacitance/charge_integration.pycharlib/characterizer/procedures/sequential/constraint/metastability/c2q_contour.pycharlib/cli/main.pycharlib/cli/run.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charlib/characterizer/procedures/combinational/delay.py`:
- Around line 194-196: Update the empty-measurement error path in the
combinational delay procedure to raise a ProcedureFailedException containing the
relevant cell or pin measurement context, consistent with earlier
simulation-failure handling. Also replace the unused meas_path binding in the
name.split('__') unpacking with an ignored variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f0a2de6d-bf2c-421c-a2e8-33ac8b3c7a40
📒 Files selected for processing (2)
charlib/characterizer/procedures/combinational/delay.pycharlib/characterizer/procedures/sequential/constraint/metastability/c2q_contour.py
4f35bf9 to
593eed5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charlib/config/syntax.py`:
- Around line 482-483: Replace the backslash-based multiline string in the
dry_run description near charlib/config/syntax.py:482-483 with parenthesized
adjacent strings, removing the doubled space before “simulations”. Also update
the ProcedureFailedException message near
charlib/characterizer/procedures/combinational/delay.py:195-196 to use
parenthesized string concatenation instead of a backslash.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 73bf934b-44f3-4984-a6fd-8e628dc90fc2
📒 Files selected for processing (2)
charlib/characterizer/procedures/combinational/delay.pycharlib/config/syntax.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charlib/characterizer/characterizer.py`:
- Line 89: Update the ProcessPoolExecutor configuration in the characterizer
execution flow to avoid recycling workers after every task: either expose
max_tasks_per_child through the existing settings configuration or increase it
to a workload-appropriate value, while preserving the current behavior only if a
documented resource leak requires it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d25cae06-9adb-4960-9a0f-35df8427dbb3
📒 Files selected for processing (3)
charlib/characterizer/characterizer.pycharlib/characterizer/procedures/combinational/leakage_power.pycharlib/liberty/liberty.py
💤 Files with no reviewable changes (1)
- charlib/characterizer/procedures/combinational/leakage_power.py
This reverts commit 3ed0a03, which didn't seem to have any effect.
|
Merged upstream in stineje#115 |
1 participant
Summary by CodeRabbit
run --no-simto skip SPICE simulations while still generating Liberty output.settings.dry_run(default:false) in config to control simulation skipping.