docs(cts): fix -sink_clustering_size/-max_diameter auto-selection description (#10602)#10726
Open
saurav-fermions wants to merge 1 commit into
Open
docs(cts): fix -sink_clustering_size/-max_diameter auto-selection description (#10602)#10726saurav-fermions wants to merge 1 commit into
saurav-fermions wants to merge 1 commit into
Conversation
…cription The README claimed the auto-selected -sink_clustering_size is chosen between 10, 20 or 30 (and -sink_clustering_max_diameter between 50, 100 or 200) based on the tree buffer max cap. No such candidate set exists in the code. When the option is not set, clusters are grown under a capacitance limit (sink buffer input cap * 10) and the resulting maximum cluster size/diameter is reported via SinkClustering::getMaxSize/ getMaxDiameter, which can be any data-dependent positive integer (e.g. 8 observed on IHP SG13G2). Correct the description to match the real behavior. Docs-only change; no code behavior modified. Fixes The-OpenROAD-Project#10602 Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the documentation in src/cts/README.md to clarify the automatic selection behavior of -sink_clustering_size and -sink_clustering_max_diameter when they are not explicitly specified. The review feedback focuses on improving grammatical correctness, readability, and consistency within the newly added text (such as hyphenating 'data-dependent' and expanding 'max fanout' to 'maximum fanout'). Additionally, the reviewer noted a duplicated -sink_clustering_levels entry in the document that should be cleaned up.
| | `-sink_clustering_enable` | Enables pre-clustering of sinks to create one level of sub-tree before building H-tree. Each cluster is driven by buffer which becomes end point of H-tree structure. | | ||
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the size will be automatically chosen between `10, 20 or 30` based on the tree buffer max cap. | | ||
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the diameter will be automatically chosen between `50, 100 or 200`, based on the tree buffer max cap. | | ||
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the size is not used as a hard limit; instead clusters are grown subject to a capacitance limit (the sink buffer input capacitance times an internal factor of 10), and the resulting maximum cluster size is selected automatically. The auto-selected value can be any positive integer and is data dependent (it is not restricted to a fixed set of values such as 10/20/30); it is bounded above by an internal cap (default 30, or the sink buffer max fanout when that is smaller). | |
Contributor
There was a problem hiding this comment.
To improve readability and grammatical correctness, consider adding a comma after "instead", hyphenating "data-dependent", and using "maximum fanout" instead of "max fanout" for consistency with the rest of the document.
Suggested change
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the size is not used as a hard limit; instead clusters are grown subject to a capacitance limit (the sink buffer input capacitance times an internal factor of 10), and the resulting maximum cluster size is selected automatically. The auto-selected value can be any positive integer and is data dependent (it is not restricted to a fixed set of values such as 10/20/30); it is bounded above by an internal cap (default 30, or the sink buffer max fanout when that is smaller). | | |
| | -sink_clustering_size | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers [0, MAX_INT]. If this is not specified, the size is not used as a hard limit; instead, clusters are grown subject to a capacitance limit (the sink buffer input capacitance times an internal factor of 10), and the resulting maximum cluster size is selected automatically. The auto-selected value can be any positive integer and is data-dependent (it is not restricted to a fixed set of values such as 10/20/30); it is bounded above by an internal cap (default 30, or the sink buffer maximum fanout when that is smaller). | |
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the size will be automatically chosen between `10, 20 or 30` based on the tree buffer max cap. | | ||
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the diameter will be automatically chosen between `50, 100 or 200`, based on the tree buffer max cap. | | ||
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the size is not used as a hard limit; instead clusters are grown subject to a capacitance limit (the sink buffer input capacitance times an internal factor of 10), and the resulting maximum cluster size is selected automatically. The auto-selected value can be any positive integer and is data dependent (it is not restricted to a fixed set of values such as 10/20/30); it is bounded above by an internal cap (default 30, or the sink buffer max fanout when that is smaller). | | ||
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the diameter is not used as a hard limit; clusters are grown subject to the same capacitance limit and the resulting maximum cluster diameter is selected automatically. The auto-selected value is data dependent and is not restricted to a fixed set of values such as 50/100/200. | |
Contributor
There was a problem hiding this comment.
To improve grammatical correctness, consider adding a comma before the coordinating conjunction "and" when connecting two independent clauses, and hyphenating "data-dependent".
Suggested change
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the diameter is not used as a hard limit; clusters are grown subject to the same capacitance limit and the resulting maximum cluster diameter is selected automatically. The auto-selected value is data dependent and is not restricted to a fixed set of values such as 50/100/200. | | |
| | -sink_clustering_max_diameter | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers [0, MAX_INT]. If this is not specified, the diameter is not used as a hard limit; clusters are grown subject to the same capacitance limit, and the resulting maximum cluster diameter is selected automatically. The auto-selected value is data-dependent and is not restricted to a fixed set of values such as 50/100/200. | |
| | `-sink_clustering_levels` | ... | | ||
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the diameter will be automatically chosen between `50, 100 or 200`, based on the tree buffer max cap. | | ||
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the size will be automatically chosen between `10, 20 or 30` based on the tree buffer max cap. | | ||
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the diameter is not used as a hard limit; clusters are grown subject to the same capacitance limit and the resulting maximum cluster diameter is selected automatically. The auto-selected value is data dependent and is not restricted to a fixed set of values such as 50/100/200. | |
Contributor
There was a problem hiding this comment.
To improve grammatical correctness, consider adding a comma before the coordinating conjunction "and" when connecting two independent clauses, and hyphenating "data-dependent".
Additionally, please note that -sink_clustering_levels is duplicated on lines 180 and 183 of this file, which should be cleaned up.
Suggested change
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the diameter is not used as a hard limit; clusters are grown subject to the same capacitance limit and the resulting maximum cluster diameter is selected automatically. The auto-selected value is data dependent and is not restricted to a fixed set of values such as 50/100/200. | | |
| | -sink_clustering_max_diameter | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers [0, MAX_INT]. If this is not specified, the diameter is not used as a hard limit; clusters are grown subject to the same capacitance limit, and the resulting maximum cluster diameter is selected automatically. The auto-selected value is data-dependent and is not restricted to a fixed set of values such as 50/100/200. | |
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the diameter will be automatically chosen between `50, 100 or 200`, based on the tree buffer max cap. | | ||
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified the size will be automatically chosen between `10, 20 or 30` based on the tree buffer max cap. | | ||
| | `-sink_clustering_max_diameter` | Specifies maximum diameter (in microns) of sink cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the diameter is not used as a hard limit; clusters are grown subject to the same capacitance limit and the resulting maximum cluster diameter is selected automatically. The auto-selected value is data dependent and is not restricted to a fixed set of values such as 50/100/200. | | ||
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the size is not used as a hard limit; instead clusters are grown subject to a capacitance limit (the sink buffer input capacitance times an internal factor of 10), and the resulting maximum cluster size is selected automatically. The auto-selected value can be any positive integer and is data dependent (it is not restricted to a fixed set of values such as 10/20/30); it is bounded above by an internal cap (default 30, or the sink buffer max fanout when that is smaller). | |
Contributor
There was a problem hiding this comment.
To improve readability and grammatical correctness, consider adding a comma after "instead", hyphenating "data-dependent", and using "maximum fanout" instead of "max fanout" for consistency with the rest of the document.
Suggested change
| | `-sink_clustering_size` | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers `[0, MAX_INT]`. If this is not specified, the size is not used as a hard limit; instead clusters are grown subject to a capacitance limit (the sink buffer input capacitance times an internal factor of 10), and the resulting maximum cluster size is selected automatically. The auto-selected value can be any positive integer and is data dependent (it is not restricted to a fixed set of values such as 10/20/30); it is bounded above by an internal cap (default 30, or the sink buffer max fanout when that is smaller). | | |
| | -sink_clustering_size | Specifies the maximum number of sinks per cluster for the register tree. The allowed values are integers [0, MAX_INT]. If this is not specified, the size is not used as a hard limit; instead, clusters are grown subject to a capacitance limit (the sink buffer input capacitance times an internal factor of 10), and the resulting maximum cluster size is selected automatically. The auto-selected value can be any positive integer and is data-dependent (it is not restricted to a fixed set of values such as 10/20/30); it is bounded above by an internal cap (default 30, or the sink buffer maximum fanout when that is smaller). | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/cts/README.mdclaimed-sink_clustering_size, when unspecified, is 'chosen between 10, 20 or 30' — there is no such {10,20,30} candidate set in the CTS code. This corrects the doc to describe the actual capacitance-limited auto-selection.Type of Change
Impact
Documentation only.
Verification
Related Issues
Fixes #10602
Developed with SAIGE, Fermions' autonomous RTL/EDA debugging agent; root-caused, tested, and signed off by the submitter (@saurav-fermions).