Conversation
Signed-off-by: GitHub Actions <actions@github.com>
# Conflicts: # bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp
Signed-off-by: GitHub Actions <actions@github.com>
Signed-off-by: GitHub Actions <actions@github.com>
Signed-off-by: GitHub Actions <actions@github.com>
Signed-off-by: GitHub Actions <actions@github.com>
Signed-off-by: GitHub Actions <actions@github.com>
Signed-off-by: GitHub Actions <actions@github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
17499-17499: Consider expandingpo_orderdocumentation.The docstring for
po_orderis minimal. Per the past review suggestion, consider clarifying its purpose: e.g., "Explicit primary output order to use whenoutput_order == RANDOM_PO_ORDER. If empty, an internal random permutation is chosen."
6075-6077: Empty docstrings forconnect_and_placehelpers remain unaddressed.These empty docstrings were flagged in a previous review. If these are internal helpers not intended for Python users, consider excluding them from the bindings. Otherwise, add at least a brief description of their purpose.
🧹 Nitpick comments (4)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (1)
1-3: Migrate "Created by" comment to Doxygen tags.Per coding guidelines, use
@fileand@authortags in Doxygen comments instead of// Created by ...comments.-// -// Created by benjamin on 21.01.25. -// +/** + * @file planar_layout_from_network_embedding.hpp + * @brief Planar layout generation from network embedding (PLANE) algorithm. + * @author Benjamin Hibler + */include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (2)
1-3: Migrate "Created by" comment to Doxygen tags.Per coding guidelines, use
@fileand@authortags in Doxygen comments instead of// Created by ...comments.-// -// Created by benjamin on 11.06.24. -// +/** + * @file node_duplication_planarization.hpp + * @brief Node duplication planarization for crossing minimization (NDCE). + * @author Benjamin Hibler + */
297-304: Consider runtime validation instead of assert for fanin arity.The assert at lines 297-299 is compiled out in release builds. For robustness, consider throwing an exception on mismatch to catch logic errors at runtime.
- // Ensure child count matches function arity (including 0-fanin constants) - assert(children.size() == ntk.fanin_size(nd) && - "Mismatch between gathered children and node fanin count"); + // Ensure child count matches function arity (including 0-fanin constants) + if (children.size() != ntk.fanin_size(nd)) + { + throw std::runtime_error("Mismatch between gathered children and node fanin count"); + }cli/cmd/physical_design/src/plane.cpp (1)
1-3: Migrate "Created by" comment to Doxygen tags.Per coding guidelines, use
@fileand@authortags in Doxygen comments.-// -// Created by benjamin on 21.11.25. -// +/** + * @file plane.cpp + * @brief CLI command for planar embedding and layout generation. + * @author Benjamin Hibler + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(11 hunks)cli/cmd/physical_design/src/plane.cpp(1 hunks)include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp(1 hunks)include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp(1 hunks)test/algorithms/physical_design/planar_layout_from_network_embedding.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,hpp,cc,cxx,h}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{cpp,hpp,cc,cxx,h}: Usesnake_casefor namespaces, functions, variables, classes, structs, and filenames in C++
UsePascalCasefor C++ template parameters (e.g.,template <typename Spec, typename Impl>)
UseUPPER_SNAKE_CASEfor C++ macros
Apply Doxygen documentation (/** ... */) to ALL new and edited C++ code symbols (members, aliases, functions, classes, etc.)
Use modern Doxygen commands (@brief,@param,@return,@tparam,@file,@author,@ref,@see,@throws) in C++ documentation
Plan migration away from// Created by ...comments in C++ files; use@fileand@authortags in Doxygen comments instead
Useconstcorrectness throughout C++ code
Prefer STL algorithms over custom algorithms in C++
Use braced initialization in C++
Never useusing namespace std;in C++ code
Files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpptest/algorithms/physical_design/planar_layout_from_network_embedding.cppinclude/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hppinclude/fiction/algorithms/network_transformation/node_duplication_planarization.hppcli/cmd/physical_design/src/plane.cpp
**/*.hpp
📄 CodeRabbit inference engine (AGENTS.md)
Use
.hppextension for C++ header files and include guards with#ifndef FICTION_FILENAME_HPPformat
Files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hppinclude/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
🧠 Learnings (6)
📓 Common learnings
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
📚 Learning: 2025-09-25T10:31:01.287Z
Learnt from: marcelwa
Repo: cda-tum/fiction PR: 840
File: bindings/mnt/pyfiction/CMakeLists.txt:30-37
Timestamp: 2025-09-25T10:31:01.287Z
Learning: Headers in the pyfiction Python bindings target (bindings/mnt/pyfiction/) are private implementation details for the Python bindings and should not be made public or exported as part of the C++ API.
Applied to files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp
📚 Learning: 2025-10-13T14:43:13.479Z
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
Applied to files:
test/algorithms/physical_design/planar_layout_from_network_embedding.cppinclude/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hppinclude/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
📚 Learning: 2025-09-10T16:31:45.364Z
Learnt from: marcelwa
Repo: cda-tum/fiction PR: 828
File: include/fiction/technology/area.hpp:0-0
Timestamp: 2025-09-10T16:31:45.364Z
Learning: In include/fiction/technology/area.hpp, both fiction::area overloads should accept parameters as `const area_params<technology<Lyt>>& ps = {}` to allow binding the default temporary and ensure const-correctness. Docs in docs/technology/properties.rst should mirror these signatures.
Applied to files:
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
📚 Learning: 2025-12-03T11:45:06.423Z
Learnt from: CR
Repo: cda-tum/fiction PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T11:45:06.423Z
Learning: Applies to **/*.{cpp,hpp,cc,cxx,h} : Use `const` correctness throughout C++ code
Applied to files:
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
📚 Learning: 2025-12-03T11:45:06.423Z
Learnt from: CR
Repo: cda-tum/fiction PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T11:45:06.423Z
Learning: Applies to **/*.{cpp,hpp,cc,cxx,h} : Use braced initialization in C++
Applied to files:
cli/cmd/physical_design/src/plane.cpp
🧬 Code graph analysis (2)
test/algorithms/physical_design/planar_layout_from_network_embedding.cpp (3)
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (1)
node_duplication_planarization(769-770)include/fiction/algorithms/verification/virtual_miter.hpp (1)
virtual_miter(77-77)include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (2)
plane(1144-1180)plane(1144-1145)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (2)
include/fiction/utils/placement_utils.hpp (1)
reserve_input_nodes(39-39)include/fiction/utils/network_utils.hpp (1)
high_degree_fanin_exception(255-256)
🪛 Cppcheck (2.18.0)
test/algorithms/physical_design/planar_layout_from_network_embedding.cpp
[information] 1-1: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[performance] 100-100: Function parameter 'p' should be passed by const reference.
(passedByValue)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
cli/cmd/physical_design/src/plane.cpp
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingInclude)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingInclude)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[error] 71-71: There is an unknown macro here somewhere. Configuration is required. If ALICE_ADD_STORE is a macro then please configure it.
(unknownMacro)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
🔇 Additional comments (25)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (3)
865-878: LGTM!The
run()method is well-structured with proper initialization of the layout, node mapping, and PI reservation. The assertion at line 886 correctly enforces the invariant that networks must have at least one PI, as per the project's design requirements.
107-150: LGTM!The buffer lookup table is well-documented and encapsulates the orientation/spacing logic cleanly. Using a static local variable ensures single initialization.
1143-1180: LGTM!The public
plane()function has proper static assertions, validates planarity viamincross, and correctly propagates statistics when requested.include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (5)
68-98: LGTM!The
hgraph_nodestruct is well-documented with proper Doxygen comments. The constructor usesconst¶meters as recommended.
114-137: LGTM!The
initialize_copy_network_duplicatesfunction has proper static assertions with accurate messages, and correctly handles constant mapping and PI creation.
153-215: LGTM!The
gather_fanin_signalsfunction correctly handles type consistency between source and destination networks, and properly preserves complemented edges viacreate_not.
631-723: LGTM!The
run()method correctly orchestrates level-by-level processing, handles PO ordering (with optional randomization), and properly constructs the virtual PI network with restored names.
768-797: LGTM!The public
node_duplication_planarizationfunction has proper type constraints, validates balance requirements, and verifies planarity of the result.test/algorithms/physical_design/planar_layout_from_network_embedding.cpp (4)
28-34: LGTM!The
check_statshelper function correctly validates that the layout statistics are populated with non-zero values.
36-69: LGTM!The
check_planetemplate function provides comprehensive testing of the full pipeline: fanout substitution → balancing → planarization → equivalence checking → layout generation → gate library application.
71-88: LGTM!The exception tests properly verify that the algorithm rejects networks with unsupported fanin degrees and non-planar inputs.
107-140: LGTM!The name conservation test verifies that network and output names correctly propagate through balancing, planarization, and layout generation stages.
cli/cmd/physical_design/src/plane.cpp (3)
32-47: LGTM!The constructor provides a detailed help string explaining the algorithm's workflow and its trade-offs compared to the gold algorithm. Options are well-organized.
49-123: LGTM!The
execute()method has proper validation, uses braced scopes in the switch statement, handles exceptions appropriately, and resets state after execution.
125-132: LGTM!The
log()method returns comprehensive statistics in JSON format, including runtime, gate/wire counts, crossings, and layout dimensions with computed area.bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (10)
5035-5053: LGTM!The docstring for
adjust_final_valuesis well-structured with clear parameter descriptions and purpose explanation.
5240-5298: LGTM!The calculation helper functions are well-documented with clear return value semantics and parameter descriptions.
5326-5412: LGTM!Documentation for these calculation helpers is comprehensive with proper parameter and return value descriptions.
6109-6145: LGTM!The docstring now properly clarifies
ntk_lvlsas containing original node ranks from the source network, documentsntk_lvls_newcorrectly, and includes aReturns:section describing the constructedvirtual_pi_network. This addresses the previous review concerns.
8021-8033: LGTM!The lookup table function docstrings adequately describe their purpose and the encoding scheme.
8918-8945: LGTM!The
hgraph_nodestruct documentation clearly explains its role in crossing minimization and documents the constructor parameters.
9526-9581: LGTM!The
compute_slice_delaysandinsert_if_not_firstdocstrings are now comprehensive. Notably,insert_if_not_firstcorrectly clarifies that "No exception is thrown during this process," addressing the previous review concern about inaccurate exception semantics.
10183-10207: LGTM!The
plane_implclass is well-documented with a clear description of the planar layout generation algorithm, including its level-by-level placement and routing approach.
17457-17489: LGTM!The main
node_duplication_planarizationfunction is thoroughly documented with proper academic citation, a clear explanation of the NDCE problem and H-graph approach, and complete parameter/return documentation.
18830-18887: LGTM!The documentation for
planar_layout_from_network_embedding_params,planar_layout_from_network_embedding_stats, and the mainplanefunction is comprehensive. The PLANE methodology is clearly explained with proper template and parameter documentation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (1)
1-3: Migrate to Doxygen file documentation.Replace the "Created by" comment with Doxygen
@fileand@authortags as per project coding guidelines.Apply this diff:
-// -// Created by benjamin on 21.01.25. -// +/** + * @file planar_layout_from_network_embedding.hpp + * @brief Planar layout generation from network embedding (PLANE algorithm) + * @author benjamin + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,hpp,cc,cxx,h}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{cpp,hpp,cc,cxx,h}: Usesnake_casefor namespaces, functions, variables, classes, structs, and filenames in C++
UsePascalCasefor C++ template parameters (e.g.,template <typename Spec, typename Impl>)
UseUPPER_SNAKE_CASEfor C++ macros
Apply Doxygen documentation (/** ... */) to ALL new and edited C++ code symbols (members, aliases, functions, classes, etc.)
Use modern Doxygen commands (@brief,@param,@return,@tparam,@file,@author,@ref,@see,@throws) in C++ documentation
Plan migration away from// Created by ...comments in C++ files; use@fileand@authortags in Doxygen comments instead
Useconstcorrectness throughout C++ code
Prefer STL algorithms over custom algorithms in C++
Use braced initialization in C++
Never useusing namespace std;in C++ code
Files:
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
**/*.hpp
📄 CodeRabbit inference engine (AGENTS.md)
Use
.hppextension for C++ header files and include guards with#ifndef FICTION_FILENAME_HPPformat
Files:
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
🧠 Learnings (2)
📓 Common learnings
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
📚 Learning: 2025-10-13T14:43:13.479Z
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
Applied to files:
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
🧬 Code graph analysis (1)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (2)
include/fiction/utils/placement_utils.hpp (1)
reserve_input_nodes(39-39)include/fiction/utils/network_utils.hpp (1)
high_degree_fanin_exception(255-256)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 🍎 macos-14 with clang++
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-16
- GitHub Check: 🐧 ubuntu-24.04 with clang++-16
- GitHub Check: 🐧 ubuntu-24.04 with g++-12
- GitHub Check: 🪟 windows-2022 with v143 toolset
- GitHub Check: 🪟 windows-2025 with v143 toolset
- GitHub Check: 🪟 windows-2022 with ClangCL toolset
- GitHub Check: 🪟 windows-2025 with ClangCL toolset
- GitHub Check: 🛞 Wheels for ubuntu-24.04-arm
- GitHub Check: 🛞 Wheels for windows-latest
- GitHub Check: 🛞 Wheels for ubuntu-24.04
- GitHub Check: 🛞 Wheels for macos-15
- GitHub Check: Analyze python
- GitHub Check: Analyze cpp
- GitHub Check: 🐳 Build and publish Docker image
- GitHub Check: Coverage on ubuntu-24.04 with g++-13
- GitHub Check: 🚨 Clang-Tidy
- GitHub Check: 🐍 macos-15
- GitHub Check: 🐍 windows-latest
- GitHub Check: 🐍 ubuntu-latest
🔇 Additional comments (1)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (1)
45-55: Add Doxygen documentation to struct members.Per coding guidelines, all struct members require Doxygen documentation. The
planar_layout_from_network_embedding_statsstruct (lines 59-98) demonstrates the expected format.Apply this diff:
struct planar_layout_from_network_embedding_params { /** * Number of clock phases to use. 3 and 4 are supported. */ num_clks number_of_clock_phases = num_clks::FOUR; /** * Verbosity. */ bool verbose = false; };Based on coding guidelines.
⛔ Skipped due to learnings
Learnt from: CR Repo: cda-tum/fiction PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-03T11:45:06.423Z Learning: Applies to **/*.{cpp,hpp,cc,cxx,h} : Use modern Doxygen commands (`brief`, `param`, `return`, `tparam`, `file`, `author`, `ref`, `see`, `throws`) in C++ documentation
marcelwa
left a comment
marcelwa
left a comment
There was a problem hiding this comment.
Thanks for addressing the first batch of comments so swiftly. I have now managed to look at the remainder of the changes, too. Looks pretty good overall. Please find my comments below.
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
Outdated
Show resolved
Hide resolved
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
Outdated
Show resolved
Hide resolved
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
Outdated
Show resolved
Hide resolved
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
Outdated
Show resolved
Hide resolved
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
Show resolved
Hide resolved
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
Show resolved
Hide resolved
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
6075-6077: Empty docstrings for connect_and_place helpers remain unaddressed.These helper functions still have empty docstrings
R"doc()doc", despite being flagged in previous reviews. Please add at least one-line descriptions of their purpose, parameters, and side effects, or exclude them from the Python bindings if they are internal implementation details not intended for Python users.Based on past review comments, as these are internal helpers for placement and routing, minimal documentation such as the following would suffice:
-static const char *__doc_fiction_detail_connect_and_place = R"doc()doc"; +static const char *__doc_fiction_detail_connect_and_place = +R"doc(Connects network nodes within a level according to the computed placement +and updates the layout accordingly. Internal helper.)doc"; -static const char *__doc_fiction_detail_connect_and_place_2 = R"doc()doc"; +static const char *__doc_fiction_detail_connect_and_place_2 = +R"doc(Variant of connect_and_place handling alternative routing cases. Internal +helper; not part of the public API.)doc";
9526-9553: Missingborder_pisparameter documentation.The docstring for
compute_slice_delaysdocuments only thendparameter but omits thebool border_pisflag. According to past review comments, this function takes aborder_pisparameter (seeinclude/fiction/algorithms/network_transformation/detail/node_duplication_planarization.hpp, Line 451). Please add documentation explaining how this boolean flag influences the slice delay calculation.Suggested addition:
Parameter ``nd``: - Node in the H-graph.)doc"; + Node in the H-graph. + +Parameter ``border_pis``: + A boolean indicating whether the input PIs (Primary Inputs) should + be propagated to the next level.)doc";include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (1)
305-307: Replace assert with runtime error check.Asserts are compiled out in release builds. Use a runtime check to ensure
create_nodeis never called with wrong arity, which would cause undefined behavior.🔎 Apply this diff:
- // Ensure child count matches function arity (including 0-fanin constants) - assert(children.size() == ntk.fanin_size(nd) && - "Mismatch between gathered children and node fanin count"); + // Ensure child count matches function arity (including 0-fanin constants) + if (children.size() != ntk.fanin_size(nd)) + { + throw std::runtime_error("Mismatch between gathered children and node fanin count"); + }
🧹 Nitpick comments (5)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
9524-9524: Empty docstring for implementation class.The
node_duplication_planarization_implclass has an empty docstring. Consider adding a brief description of the class's role in the planarization algorithm, or mark this as an internal implementation detail not exposed to Python users.
17504-17512: Consider enhancingpo_orderparameter description.The
po_orderparameter is documented but could be more explicit about when and how it's used. The current description states it defaults toKEEP_PO_ORDER, but doesn't clarify its role whenoutput_order == RANDOM_PO_ORDER.Consider expanding the description:
-static const char *__doc_fiction_node_duplication_planarization_params_po_order = R"doc(The output order used. Defaults to KEEP_PO_ORDER.)doc"; +static const char *__doc_fiction_node_duplication_planarization_params_po_order = +R"doc(Explicit primary output order to use when output_order == RANDOM_PO_ORDER. +If empty, an internal random permutation is chosen. Defaults to KEEP_PO_ORDER.)doc";cli/cmd/physical_design/src/plane.cpp (1)
1-3: Consider migrating to@fileand@authorDoxygen tags.Per coding guidelines, plan migration away from
// Created by ...comments in favor of Doxygen documentation.cli/cmd/physical_design/include/plane.hpp (1)
1-3: Consider migrating to@fileand@authorDoxygen tags.Per coding guidelines, plan migration away from
// Created by ...comments in favor of Doxygen documentation.include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (1)
1-3: Migrate "Created by" comment to Doxygen format.Per coding guidelines, replace the "Created by" comment with Doxygen
@fileand@authortags.🔎 Apply this diff:
-// -// Created by benjamin on 11.06.24. -// +/** + * @file node_duplication_planarization.hpp + * @author benjamin + * @brief Implements node duplication planarization for logic networks. + */
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(11 hunks)cli/cmd/physical_design/include/plane.hpp(1 hunks)cli/cmd/physical_design/src/plane.cpp(1 hunks)docs/changelog.rst(1 hunks)include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/changelog.rst
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,hpp,cc,cxx,h}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{cpp,hpp,cc,cxx,h}: Usesnake_casefor namespaces, functions, variables, classes, structs, and filenames in C++
UsePascalCasefor C++ template parameters (e.g.,template <typename Spec, typename Impl>)
UseUPPER_SNAKE_CASEfor C++ macros
Apply Doxygen documentation (/** ... */) to ALL new and edited C++ code symbols (members, aliases, functions, classes, etc.)
Use modern Doxygen commands (@brief,@param,@return,@tparam,@file,@author,@ref,@see,@throws) in C++ documentation
Plan migration away from// Created by ...comments in C++ files; use@fileand@authortags in Doxygen comments instead
Useconstcorrectness throughout C++ code
Prefer STL algorithms over custom algorithms in C++
Use braced initialization in C++
Never useusing namespace std;in C++ code
Files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppcli/cmd/physical_design/include/plane.hppcli/cmd/physical_design/src/plane.cppinclude/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
**/*.hpp
📄 CodeRabbit inference engine (AGENTS.md)
Use
.hppextension for C++ header files and include guards with#ifndef FICTION_FILENAME_HPPformat
Files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppcli/cmd/physical_design/include/plane.hppinclude/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
🧠 Learnings (7)
📓 Common learnings
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
📚 Learning: 2025-09-25T10:31:01.287Z
Learnt from: marcelwa
Repo: cda-tum/fiction PR: 840
File: bindings/mnt/pyfiction/CMakeLists.txt:30-37
Timestamp: 2025-09-25T10:31:01.287Z
Learning: Headers in the pyfiction Python bindings target (bindings/mnt/pyfiction/) are private implementation details for the Python bindings and should not be made public or exported as part of the C++ API.
Applied to files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp
📚 Learning: 2025-12-03T11:45:06.423Z
Learnt from: CR
Repo: cda-tum/fiction PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T11:45:06.423Z
Learning: Applies to **/*.py : Use Google-style docstrings for Python functions and classes
Applied to files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp
📚 Learning: 2025-10-13T14:43:13.479Z
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
Applied to files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
📚 Learning: 2025-12-03T11:45:06.423Z
Learnt from: CR
Repo: cda-tum/fiction PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T11:45:06.423Z
Learning: Applies to **/*.{cpp,hpp,cc,cxx,h} : Use braced initialization in C++
Applied to files:
cli/cmd/physical_design/src/plane.cpp
📚 Learning: 2025-09-10T16:31:45.364Z
Learnt from: marcelwa
Repo: cda-tum/fiction PR: 828
File: include/fiction/technology/area.hpp:0-0
Timestamp: 2025-09-10T16:31:45.364Z
Learning: In include/fiction/technology/area.hpp, both fiction::area overloads should accept parameters as `const area_params<technology<Lyt>>& ps = {}` to allow binding the default temporary and ensure const-correctness. Docs in docs/technology/properties.rst should mirror these signatures.
Applied to files:
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
📚 Learning: 2025-12-03T11:45:06.423Z
Learnt from: CR
Repo: cda-tum/fiction PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T11:45:06.423Z
Learning: Applies to **/*.{cpp,hpp,cc,cxx,h} : Use `const` correctness throughout C++ code
Applied to files:
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
🧬 Code graph analysis (1)
cli/cmd/physical_design/src/plane.cpp (3)
cli/cmd/physical_design/include/plane.hpp (1)
plane_command(34-34)include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (1)
node_duplication_planarization(810-811)include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (2)
plane(1145-1181)plane(1145-1146)
🪛 Cppcheck (2.18.0)
cli/cmd/physical_design/src/plane.cpp
[error] 71-71: There is an unknown macro here somewhere. Configuration is required. If ALICE_ADD_STORE is a macro then please configure it.
(unknownMacro)
🔇 Additional comments (18)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (11)
5035-5053: LGTM! Well-documented function.The docstring for
adjust_final_valuesclearly explains the function's purpose, parameters, and behavior. The description effectively conveys the minimal routing demand identification logic.
5240-5298: LGTM! Comprehensive documentation.All three helper functions (
calculate_allowed_orientation,calculate_buffer_connection_type,calculate_fanout_connection_type) are thoroughly documented with clear explanations of their return codes and parameter semantics.
5326-5413: LGTM! Clear and complete documentation.All four calculation helper functions are well-documented with proper template parameter specifications, parameter descriptions, and return value explanations.
6060-6073: LGTM! Function properly documented.The
compute_two_input_indicesfunction is clearly documented with appropriate template parameters, parameters, and return value descriptions.
6109-6145: LGTM! Previous issues resolved.The docstring for
create_virtual_pi_ntk_from_duplicated_nodesnow consistently describesntk_lvlsas containing original node ranks from the source network, properly documentsntk_lvls_new, and includes a clear return value description. The previous inconsistencies have been addressed.
8021-8033: LGTM! Lookup table functions documented.Both
get_buffer_lookupandget_fanout_lookuphave clear docstrings explaining their 3D lookup table structure and purpose.
8918-8945: LGTM! H-graph node structure well-documented.The
hgraph_nodestructure and its members are thoroughly documented, including the template parameter, constructor parameters, and member variable semantics.
9564-9590: LGTM! Semantics clarified.The updated docstring for
insert_if_not_firstnow correctly states "No exception is thrown during this process" and accurately describes the insertion behavior based onsaturated_fanout_flagandposition. The previous misleading exception reference has been removed.
10200-10224: LGTM! Algorithm implementation well-documented.The
plane_implclass docstring provides a comprehensive description of the planar layout generation algorithm, including its level-by-level approach, template parameters, and parameters. The explanation clearly conveys the algorithm's purpose and operation.
17474-17502: LGTM! Algorithm thoroughly documented.The
node_duplication_planarizationfunction docstring provides excellent coverage of the algorithm, including its basis in the referenced paper, the NDCE problem it solves, the H-graph approach, and clear parameter/return documentation.
18843-18900: LGTM! PLANE methodology thoroughly documented.The
planar_layout_from_network_embedding_params, statistics struct, andplanefunction are all comprehensively documented. The docstring forplaneprovides excellent context about the PLANE methodology, its approach to preserving planar embeddings, and the mapping from logic levels to layout diagonals.cli/cmd/physical_design/src/plane.cpp (4)
32-47: LGTM!The constructor properly initializes parameters and configures CLI options with appropriate descriptions.
69-87: LGTM!The switch statement correctly maps user input to internal enum values with braced scopes and full enum names. Good defensive handling with the default case.
101-117: Verify behavior ofgls.extend()on exception.If
plane()throws, thegls.extend() = ...assignment is not completed. Depending on alice's store implementation, this could leave a dangling or null entry in the store. Consider callingplane()first, storing the result, then extending the store only on success.🔎 Suggested refactor
try { - gls.extend() = std::make_shared<fiction::cart_gate_clk_lyt>( - fiction::plane<fiction::cart_gate_clk_lyt>(planarized_ntk, ps, &st)); + auto result = fiction::plane<fiction::cart_gate_clk_lyt>(planarized_ntk, ps, &st); + gls.extend() = std::make_shared<fiction::cart_gate_clk_lyt>(std::move(result)); }
124-131: LGTM!The
log()function provides comprehensive statistics including runtime, gate/wire counts, crossings, and layout dimensions.cli/cmd/physical_design/include/plane.hpp (3)
5-16: LGTM!Include guard follows the
FICTION_*_HPPconvention and all necessary headers are included.
20-25: LGTM!The docstring clearly explains the PLANE acronym (Planar Layout from Network Embedding) and references the algorithm header for details. This addresses the previous reviewer's feedback about stating the acronym.
26-78: LGTM!The class definition follows coding guidelines with proper Doxygen documentation, braced initialization, explicit constructor, and appropriate use of the dedicated
planar_layout_from_network_embedding_statsclass (addressing the past reviewer concern about stats class reuse).
| if (num_clock_phases != 3u && num_clock_phases != 4u) | ||
| { | ||
| env->out() << "[e] only 3- and 4-phase clocking schemes are supported\n"; | ||
| ps = {}; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Inconsistent parameter reset on error path.
On invalid clock phases, only ps is reset, but fan_ps, bal_ps, and dup_ps are not. Conversely, at the end of execute() (lines 119-121), only those three are reset but not ps. Consider resetting all parameters consistently in both locations to avoid stale state across invocations.
🔎 Suggested fix
if (num_clock_phases != 3u && num_clock_phases != 4u)
{
env->out() << "[e] only 3- and 4-phase clocking schemes are supported\n";
- ps = {};
+ fan_ps = {};
+ bal_ps = {};
+ dup_ps = {};
+ ps = {};
return;
}And at end of execute():
fan_ps = {};
bal_ps = {};
dup_ps = {};
+ ps = {};
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cli/cmd/physical_design/src/plane.cpp around lines 60-65, the error path for
unsupported num_clock_phases only resets ps but leaves fan_ps, bal_ps, and
dup_ps stale; likewise at the end of execute() (around lines 119-121) only
fan_ps, bal_ps, and dup_ps are cleared while ps is left. Fix by consistently
clearing all four parameters (ps, fan_ps, bal_ps, dup_ps) in both places so any
early return or normal exit leaves no stale state.
| /** | ||
| * The output order used. Defaults to KEEP_PO_ORDER. | ||
| */ | ||
| output_order po_order = output_order::KEEP_PO_ORDER; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add Doxygen documentation for po_order member.
All struct members should have Doxygen documentation per coding guidelines.
🔎 Apply this diff:
/**
* The output order used. Defaults to KEEP_PO_ORDER.
*/
output_order po_order = output_order::KEEP_PO_ORDER;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| output_order po_order = output_order::KEEP_PO_ORDER; | |
| /** | |
| * The output order used. Defaults to KEEP_PO_ORDER. | |
| */ | |
| output_order po_order = output_order::KEEP_PO_ORDER; |
🤖 Prompt for AI Agents
In
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
around line 52, the struct member "po_order" lacks Doxygen documentation; add a
concise Doxygen comment immediately above the declaration that explains the
purpose of this member (controls the output ordering behavior), lists or
references the possible enum values (e.g., output_order::KEEP_PO_ORDER and other
options), and any default behavior or effects; use the project's Doxygen style
(/// or /** */) and keep the comment one or two short sentences.
| template <typename Ntk> | ||
| using levelized_node_order = std::vector<std::vector<mockturtle::node<Ntk>>>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add Doxygen documentation for levelized_node_order type alias.
All type aliases should have Doxygen documentation per coding guidelines.
🔎 Apply this diff:
+/**
+ * A type alias representing a levelized ordering of nodes in a network.
+ * Each inner vector contains nodes at a specific level (rank).
+ */
template <typename Ntk>
using levelized_node_order = std::vector<std::vector<mockturtle::node<Ntk>>>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| template <typename Ntk> | |
| using levelized_node_order = std::vector<std::vector<mockturtle::node<Ntk>>>; | |
| /** | |
| * A type alias representing a levelized ordering of nodes in a network. | |
| * Each inner vector contains nodes at a specific level (rank). | |
| */ | |
| template <typename Ntk> | |
| using levelized_node_order = std::vector<std::vector<mockturtle::node<Ntk>>>; |
🤖 Prompt for AI Agents
In
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
around lines 58 to 59, the type alias levelized_node_order lacks Doxygen
documentation; add a brief Doxygen comment immediately above the alias that
explains what the alias represents (a vector of levels where each level is a
vector of mockturtle::node<Ntk>), documents the template parameter Ntk, and
describes intended use/semantics (e.g., ordering of nodes by level for
duplication/planarization algorithms) following project Doxygen style.
| hgraph_node(const mockturtle::node<Ntk>& first, const mockturtle::node<Ntk>& last, const uint64_t delay_value) : | ||
| outer_fanins(first, last), | ||
| delay(delay_value) | ||
| {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use const references for constructor parameters.
Per const correctness guidelines, node parameters should be const &.
🔎 Apply this diff:
- hgraph_node(const mockturtle::node<Ntk>& first, const mockturtle::node<Ntk>& last, const uint64_t delay_value) :
+ hgraph_node(const mockturtle::node<Ntk>& first, const mockturtle::node<Ntk>& last, uint64_t delay_value) :
outer_fanins(first, last),
delay(delay_value)
{}Note: uint64_t is a primitive type and should be passed by value, not by const reference. Only the node parameters benefit from const & (which they already have).
🤖 Prompt for AI Agents
In
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
around lines 97 to 100, the constructor currently declares the integer parameter
as const uint64_t delay_value; change it to uint64_t delay_value (remove the
const) so the primitive is passed by value while keeping the
mockturtle::node<Ntk> parameters as const references; leave the initializer list
and member assignments unchanged.
| { | ||
| if (!ntk.is_constant(fi)) | ||
| { | ||
| auto n = ntk.get_node(fi); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Rename shadowing variable to avoid confusion.
Line 438 declares a local variable n that shadows the function parameter n (line 423), which can lead to bugs and confusion.
🔎 Apply this diff:
- auto n = ntk.get_node(fi);
+ const auto fi_node = ntk.get_node(fi);
auto it =
- std::lower_bound(fis.begin(), fis.end(), n, [&](auto const& a, auto const& b)
+ std::lower_bound(fis.begin(), fis.end(), fi_node, [&](auto const& a, auto const& b)
{ return ntk.rank_position(a) < ntk.rank_position(b); });
- fis.insert(it, n);
+ fis.insert(it, fi_node);🤖 Prompt for AI Agents
In
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
around line 438, the local variable declared as "n" shadows the function
parameter "n" (declared at line 423); rename the local variable to a
non-conflicting, descriptive name (e.g., fi_node, child_node or node_local) and
update all subsequent references at and after line 438 to use the new name so
the parameter is no longer shadowed.
| // Generate a random engine | ||
| static std::mt19937_64 generator(std::random_device{}()); | ||
| // Shuffle the pos vector | ||
| std::shuffle(pos.begin(), pos.end(), generator); |
There was a problem hiding this comment.
Document thread-safety assumptions for concurrent calls to this method.
A single PRNG across multiple threads without locking is not thread-safe. The static generator is accessed without synchronization, which could cause data races if run() is called concurrently from multiple threads. Either document that this algorithm is not thread-safe, or protect generator access with a mutex/lock or use thread-local storage for thread-safety.
🤖 Prompt for AI Agents
In
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
around lines 694-697, the static std::mt19937_64 generator is used without
synchronization which is unsafe if run() is called concurrently; either document
that this method is not thread-safe or make generator access safe — preferred
fixes: replace the static generator with a function-local generator seeded
per-call (e.g., construct a local std::mt19937_64 seeded from std::random_device
or a thread-safe seed source), or declare the generator thread_local, or guard
access with a mutex around std::shuffle; update comments to state the
thread-safety choice you make.
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
6075-6077: Empty docstrings remain forconnect_and_placehelpers.These empty docstrings were already flagged in a previous review. The functions should either be documented or excluded from Python bindings if they are internal helpers not intended for end users.
9512-9539: Missingborder_pisparameter documentation.Previous reviews flagged that
compute_slice_delaystakes abool border_pisparameter that should be documented. The current docstring only documents thendparameter, leaving theborder_pisparameter unexplained.
🧹 Nitpick comments (9)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (9)
1-6: Consider migrating to Doxygen-style file documentation.As per the coding guidelines, replace the
// Created by ...comment with@fileand@authorDoxygen tags.Suggested change:
-// -// Created by benjamin on 21.01.25. -// - +/** + * @file planar_layout_from_network_embedding.hpp + * @brief Planar layout generation from network embedding. + * @author Benjamin Hien + */ + #ifndef FICTION_PLANAR_LAYOUT_FROM_NETWORK_EMBEDDING_HPP #define FICTION_PLANAR_LAYOUT_FROM_NETWORK_EMBEDDING_HPP
256-258: Use proper node type in lambda comparator.The lambda comparator uses
intfor node parameters, butmockturtle::node<Ntk>may be a larger unsigned type. This could cause truncation or unexpected comparison behavior.Suggested change:
auto fo = ntk.fanout(n); assert(fo.size() == 2); - std::sort(fo.begin(), fo.end(), [&ntk](int a, int b) { return ntk.rank_position(a) < ntk.rank_position(b); }); + std::sort(fo.begin(), fo.end(), [&ntk](const auto& a, const auto& b) { return ntk.rank_position(a) < ntk.rank_position(b); });
300-325: Use proper types in lambda and return statement.
- The lambda comparator uses
intfor node parameters.- The return statement uses
std::min<uint64_t>but the function returnsuint8_t, causing implicit narrowing.Suggested changes:
if (fc.fanin_nodes.size() == 2) { std::sort(fc.fanin_nodes.begin(), fc.fanin_nodes.end(), - [&ntk](int a, int b) { return ntk.rank_position(a) < ntk.rank_position(b); }); + [&ntk](const auto& a, const auto& b) { return ntk.rank_position(a) < ntk.rank_position(b); }); }assert(pre1_t.y > pre2_t.y); - return std::min<uint64_t>(pre1_t.y - pre2_t.y - 1, 2); + return static_cast<uint8_t>(std::min<uint64_t>(pre1_t.y - pre2_t.y - 1, 2));
337-358: Consider returningbooland fix lambda parameter types.The function only returns 0 or 1, so
boolis more appropriate thanuint64_t. Additionally, the lambda comparator should use proper node types.Suggested changes:
template <typename Ntk> -uint64_t calculate_buffer_connection_type(const Ntk& ntk, const mockturtle::node<Ntk>& n) +bool calculate_buffer_connection_type(const Ntk& ntk, const mockturtle::node<Ntk>& n) { if (ntk.is_po(n)) { - return 0; + return false; } auto fo = ntk.fanout(n); assert(fo.size() == 1); if (ntk.fanin_size(fo[0]) == 2) { auto fc = fanins(ntk, fo[0]); auto pre = fc.fanin_nodes; assert(pre.size() == 2); - std::sort(pre.begin(), pre.end(), [&ntk](int a, int b) { return ntk.rank_position(a) < ntk.rank_position(b); }); + std::sort(pre.begin(), pre.end(), [&ntk](const auto& a, const auto& b) { return ntk.rank_position(a) < ntk.rank_position(b); }); if (pre[1] == n) { - return 1; + return true; } } - return 0; + return false; }Note: If you change the return type, you'll also need to update the lookup table indexing at line 527-529 to cast the boolean to an integer index.
383-385: Use proper node types in lambda comparator.Suggested change:
auto fo = ntk.fanout(pre); assert(fo.size() == 2); - std::sort(fo.begin(), fo.end(), [&ntk](int a, int b) { return ntk.rank_position(a) < ntk.rank_position(b); }); + std::sort(fo.begin(), fo.end(), [&ntk](const auto& a, const auto& b) { return ntk.rank_position(a) < ntk.rank_position(b); });
411-412: Consider usinguint8_treturn type for consistency.The function only returns values 0, 1, or 3 (via the expression at line 446). Using
uint8_twould be consistent with other similar helper functions.
558-565: Remove unused lambda capture.The lambda captures
lvlby reference but never uses it.Suggested change:
ntk.foreach_node_in_rank(lvl, - [&ntk, &two_input_indices, &lvl](const auto& n, const auto& i) + [&ntk, &two_input_indices](const auto& n, const auto& i) { if (ntk.fanin_size(n) == 2) { two_input_indices.emplace_back(i); } });
591-594: Use proper node types in lambda comparator.Suggested change:
if (fc.fanin_nodes.size() == 2) { - std::sort(fc.fanin_nodes.begin(), fc.fanin_nodes.end(), [&ntk](int a, int b) + std::sort(fc.fanin_nodes.begin(), fc.fanin_nodes.end(), [&ntk](const auto& a, const auto& b) { return ntk.rank_position(a) < ntk.rank_position(b); });
1030-1032: Consider usingboolforadd_line.The variable
add_lineonly holds values 0 or 1, soboolwould be more semantically appropriate.Suggested change:
// place and route POs mockturtle::node_map<uint8_t, Ntk> count_map{ntk, 0}; - int add_line = 0; + bool add_line = false;Then update line 1049 to
add_line = true;and line 1079 to useadd_line ? 1 : 0or cast appropriately.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(10 hunks)include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,hpp,cc,cxx,h}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{cpp,hpp,cc,cxx,h}: Usesnake_casefor namespaces, functions, variables, classes, structs, and filenames in C++
UsePascalCasefor C++ template parameters (e.g.,template <typename Spec, typename Impl>)
UseUPPER_SNAKE_CASEfor C++ macros
Apply Doxygen documentation (/** ... */) to ALL new and edited C++ code symbols (members, aliases, functions, classes, etc.)
Use modern Doxygen commands (@brief,@param,@return,@tparam,@file,@author,@ref,@see,@throws) in C++ documentation
Plan migration away from// Created by ...comments in C++ files; use@fileand@authortags in Doxygen comments instead
Useconstcorrectness throughout C++ code
Prefer STL algorithms over custom algorithms in C++
Use braced initialization in C++
Never useusing namespace std;in C++ code
Files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
**/*.hpp
📄 CodeRabbit inference engine (AGENTS.md)
Use
.hppextension for C++ header files and include guards with#ifndef FICTION_FILENAME_HPPformat
Files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
🧠 Learnings (5)
📓 Common learnings
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
📚 Learning: 2025-09-25T10:31:01.287Z
Learnt from: marcelwa
Repo: cda-tum/fiction PR: 840
File: bindings/mnt/pyfiction/CMakeLists.txt:30-37
Timestamp: 2025-09-25T10:31:01.287Z
Learning: Headers in the pyfiction Python bindings target (bindings/mnt/pyfiction/) are private implementation details for the Python bindings and should not be made public or exported as part of the C++ API.
Applied to files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp
📚 Learning: 2025-12-03T11:45:06.423Z
Learnt from: CR
Repo: cda-tum/fiction PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T11:45:06.423Z
Learning: Applies to **/*.py : Use Google-style docstrings for Python functions and classes
Applied to files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
📚 Learning: 2025-10-13T14:43:13.479Z
Learnt from: hibenj
Repo: cda-tum/fiction PR: 852
File: include/fiction/algorithms/physical_design/orthogonal_planar.hpp:825-826
Timestamp: 2025-10-13T14:43:13.479Z
Learning: In the fiction library, logic networks are required to have at least one primary input (PI). The `orthogonal_planar` algorithm in `include/fiction/algorithms/physical_design/orthogonal_planar.hpp` enforces this invariant with an assertion rather than handling the zero-PI case.
Applied to files:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
📚 Learning: 2025-10-13T11:04:24.736Z
Learnt from: marcelwa
Repo: cda-tum/fiction PR: 854
File: cli/cmd/logic/src/abc.cpp:78-88
Timestamp: 2025-10-13T11:04:24.736Z
Learning: The `mockturtle::write_aiger` function in the mockturtle library supports writing all kinds of logic networks (AIG, XAG, MIG, TEC), not just AIG networks. No explicit conversion to AIG is required before calling this function.
Applied to files:
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp
🔇 Additional comments (17)
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp (6)
42-98: LGTM!The parameter and statistics structs are well-documented with Doxygen comments, use braced initialization, and the
report()method provides clear formatted output.
103-231: LGTM!The lookup tables are appropriately declared as
inline constexpr, useUPPER_SNAKE_CASEnaming, and are well-documented with Doxygen comments explaining their purpose.
467-543: LGTM!The function is well-documented and correctly computes placement/routing variables using the lookup tables. The assertions properly validate array bounds.
607-828: LGTM!The wiring computation functions are well-documented. The previous concern about out-of-bounds access at line 785 has been addressed with the guard
(i < two_input_new_lines.size()) ? two_input_new_lines[i] : 0.
1039-1070: LGTM!The PO placement logic correctly addresses previous review concerns:
- Uses
ntk.get_node(po)to get the driving node- Uses
mockturtle::node_mapwith proper typing forcount_map- Properly checks for
has_output_name_vbefore accessing output names- Throws
std::logic_errorinstead ofassert(false)for the error case
1143-1180: LGTM!The public
planefunction properly:
- Validates type constraints with
static_assert- Checks for high-degree fan-in nodes
- Validates planarity using
mincross- Handles statistics output correctly
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (11)
5035-5053: LGTM!The docstring is complete, clear, and properly documents the function's purpose and all parameters.
5240-5298: LGTM!All three function docstrings are complete and well-structured, with clear descriptions of parameters, template parameters, and return values.
5326-5412: LGTM!All function docstrings are complete, well-documented, and follow a consistent format.
6060-6073: LGTM!The
compute_two_input_indicesdocstring is complete and properly documents the function's purpose, template parameter, parameters, and return value.
6109-6145: LGTM! Previous issues resolved.The docstring now properly clarifies that
ntk_lvlscontains original node ranks from the source network,ntk_lvls_newcontains newly created node ranks, and the return value is documented. The previous inconsistencies have been addressed.
8904-8932: LGTM!The H-graph node structure and its members are well-documented, with clear descriptions of purpose, fields, and constructor parameters.
9550-9576: LGTM! Previous issue resolved.The docstring has been updated to clarify that no exception is thrown, and properly explains the insertion behavior based on the
saturated_fanout_flagandpositionparameters.
9541-9549: LGTM!The enum documentation for
fanout_stateandinsertion_position, along with their values and member variables, is clear and complete.Also applies to: 9578-9591
10186-10222: LGTM!The
plane_implclass and its members are thoroughly documented, with a detailed explanation of the algorithm, template parameters, and all member variables.
17460-17498: LGTM!The
node_duplication_planarizationfunction and its parameters are well-documented. The main function includes a detailed algorithm explanation with proper citations, and all parameters and enums are documented.
18829-18886: LGTM!The
planar_layout_from_network_embeddingparameters, statistics, and mainplanefunction are comprehensively documented with clear descriptions of the PLANE methodology, all parameters, and return values.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| ntk.foreach_node_in_rank( | ||
| lvl, | ||
| [&ntk, &node2pos, &lvl, &orientation, &new_lines, &buffer_lu, &fanout_lu](const auto& n, const auto i) |
Check failure
Code scanning / CodeQL
Potentially uninitialized local variable High
|
|
||
| ntk.foreach_node_in_rank( | ||
| lvl, | ||
| [&ntk, &node2pos, &lvl, &orientation, &new_lines, &buffer_lu, &fanout_lu](const auto& n, const auto i) |
Check failure
Code scanning / CodeQL
Potentially uninitialized local variable High
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp`:
- Line 215: Fix the typo in the inline comment that currently reads "// THis
adjusts for virtual PIS" by changing "THis" to "This" so it reads "// This
adjusts for virtual PIS"; locate this comment in
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
(the line containing the exact text "THis adjusts for virtual PIS") and update
it accordingly.
- Line 129: The static_assert using mockturtle::has_rank_position_v<Ntk> is
malformed (it uses && with the message string), so replace the logical-AND
pattern with the correct static_assert syntax that takes the trait as the first
argument and the message string as the second; update the static_assert
referencing mockturtle::has_rank_position_v<Ntk> and the message "Ntk does not
implement the rank_position function" accordingly.
- Around line 844-846: The static_assert messages incorrectly reference "NtkSrc"
instead of the actual template parameter name `Ntk`; update the three
static_assert messages that mention mockturtle::is_network_type_v<Ntk>,
mockturtle::has_create_node_v<Ntk>, and mockturtle::has_rank_position_v<Ntk> to
refer to "Ntk" (e.g., "Ntk is not a network type", "Ntk does not implement the
create_node function", "Ntk does not implement the rank_position function") so
the error text matches the template parameter name used in
node_duplication_planarization.hpp.
♻️ Duplicate comments (5)
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (5)
58-59: Add Doxygen documentation forlevelized_node_ordertype alias.Per coding guidelines, all type aliases should have Doxygen documentation.
🔧 Suggested change
+/** + * `@brief` A levelized ordering of nodes in a network. + * + * Each inner vector contains nodes at a specific level (rank), ordered from outputs to inputs. + * + * `@tparam` Ntk Network type from which node types are drawn. + */ template <typename Ntk> using levelized_node_order = std::vector<std::vector<mockturtle::node<Ntk>>>;
97-100: Removeconstfrom primitive parameterdelay_value.Primitive types like
uint64_tshould be passed by value withoutconst. Theconstqualifier is unnecessary for pass-by-value primitives.🔧 Suggested change
- hgraph_node(const mockturtle::node<Ntk>& first, const mockturtle::node<Ntk>& last, const uint64_t delay_value) : + hgraph_node(const mockturtle::node<Ntk>& first, const mockturtle::node<Ntk>& last, uint64_t delay_value) :
332-339: Consider runtime check instead of assert for fanin arity.Asserts are compiled out in release builds. If
children.size()doesn't matchntk.fanin_size(nd),create_nodewill be called with incorrect arity. A runtime exception provides safer behavior.🔧 Suggested change
- // Ensure child count matches function arity (including 0-fanin constants) - assert(children.size() == ntk.fanin_size(nd) && - "Mismatch between gathered children and node fanin count"); - - const auto new_sig = ntk_dest_v.create_node(children, ntk.node_function(nd)); + // Ensure child count matches function arity (including 0-fanin constants) + if (children.size() != ntk.fanin_size(nd)) + { + throw std::runtime_error("Mismatch between gathered children and node fanin count"); + } + const auto new_sig = ntk_dest_v.create_node(children, ntk.node_function(nd));
465-471: Rename shadowing variable to avoid confusion.Line 465 declares a local variable
nthat shadows the function parametern(line 450). This can lead to bugs and confusion.🔧 Suggested change
- auto n = ntk.get_node(fi); + const auto fi_node = ntk.get_node(fi); auto it = - std::lower_bound(fis.begin(), fis.end(), n, [&](auto const& a, auto const& b) + std::lower_bound(fis.begin(), fis.end(), fi_node, [&](auto const& a, auto const& b) { return ntk.rank_position(a) < ntk.rank_position(b); }); - fis.insert(it, n); + fis.insert(it, fi_node);
725-728: Static PRNG is not thread-safe.The static
std::mt19937_64generator is accessed without synchronization. Ifrun()is called concurrently from multiple threads, this causes a data race. Consider documenting that this algorithm is not thread-safe, or usethread_localstorage.🔧 Suggested change (use thread_local)
- // Generate a random engine - static std::mt19937_64 generator(std::random_device{}()); + // Generate a random engine (thread_local for thread-safety) + thread_local std::mt19937_64 generator(std::random_device{}());
🧹 Nitpick comments (2)
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp (2)
1-4: Migrate from "Created by" comment to Doxygen@fileand@authortags.Per coding guidelines, the project is migrating away from
// Created by ...comments. Consider replacing with a Doxygen file header block.🔧 Suggested change
-// -// Created by benjamin on 11.06.24. -// +/** + * `@file` node_duplication_planarization.hpp + * `@brief` Implements node duplication planarization for crossing minimization. + * `@author` Benjamin Jiang + */
704-705: Add Doxygen documentation forrun()method.The
run()method is missing Doxygen documentation. Per coding guidelines, all public methods should be documented.🔧 Suggested change
+ /** + * `@brief` Executes the node duplication planarization algorithm. + * + * Processes the network from POs to PIs, computing optimal node orderings + * using the H-graph shortest path approach, and constructs a planar + * virtual_pi_network with duplicated nodes. + * + * `@return` A planarized virtual_pi_network containing the result. + */ [[nodiscard]] virtual_pi_network<Ntk> run()
| static_assert(mockturtle::has_foreach_pi_v<Ntk>, "Ntk does not implement the foreach_pi method"); | ||
| static_assert(mockturtle::has_create_pi_v<virtual_pi_network<Ntk>>, | ||
| "virtual_pi_network<Ntk> does not implement create_pi"); | ||
| static_assert(mockturtle::has_rank_position_v<Ntk> && "Ntk does not implement the rank_position function"); |
There was a problem hiding this comment.
Fix malformed static_assert syntax.
The static_assert on line 129 uses && instead of a comma, which will not work correctly. The expression has_rank_position_v<Ntk> && "message" evaluates to a pointer (the string literal) which is always truthy, so this assertion will never fail even if the trait is false.
🐛 Proposed fix
- static_assert(mockturtle::has_rank_position_v<Ntk> && "Ntk does not implement the rank_position function");
+ static_assert(mockturtle::has_rank_position_v<Ntk>, "Ntk does not implement the rank_position function");🤖 Prompt for AI Agents
In
`@include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp`
at line 129, The static_assert using mockturtle::has_rank_position_v<Ntk> is
malformed (it uses && with the message string), so replace the logical-AND
pattern with the correct static_assert syntax that takes the trait as the first
argument and the message string as the second; update the static_assert
referencing mockturtle::has_rank_position_v<Ntk> and the message "Ntk does not
implement the rank_position function" accordingly.
|
|
||
| // Check if the candidate matches the original fan-in or a duplicate. | ||
| // Also, verify if the candidate has already reached its fan-out limit. | ||
| // THis adjusts for virtual PIS |
There was a problem hiding this comment.
Fix typo in comment.
Minor typo: "THis" should be "This".
🔧 Suggested change
- // THis adjusts for virtual PIS
+ // This adjusts for virtual PIs📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // THis adjusts for virtual PIS | |
| // This adjusts for virtual PIs |
🤖 Prompt for AI Agents
In
`@include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp`
at line 215, Fix the typo in the inline comment that currently reads "// THis
adjusts for virtual PIS" by changing "THis" to "This" so it reads "// This
adjusts for virtual PIS"; locate this comment in
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp
(the line containing the exact text "THis adjusts for virtual PIS") and update
it accordingly.
| static_assert(mockturtle::is_network_type_v<Ntk>, "NtkSrc is not a network type"); | ||
| static_assert(mockturtle::has_create_node_v<Ntk>, "NtkSrc does not implement the create_node function"); | ||
| static_assert(mockturtle::has_rank_position_v<Ntk>, "NtkSrc does not implement the rank_position function"); |
There was a problem hiding this comment.
Fix inconsistent error message referencing NtkSrc.
The static_assert messages reference "NtkSrc" but the template parameter is named Ntk. This is a minor inconsistency.
🔧 Suggested change
- static_assert(mockturtle::is_network_type_v<Ntk>, "NtkSrc is not a network type");
- static_assert(mockturtle::has_create_node_v<Ntk>, "NtkSrc does not implement the create_node function");
- static_assert(mockturtle::has_rank_position_v<Ntk>, "NtkSrc does not implement the rank_position function");
+ static_assert(mockturtle::is_network_type_v<Ntk>, "Ntk is not a network type");
+ static_assert(mockturtle::has_create_node_v<Ntk>, "Ntk does not implement the create_node function");
+ static_assert(mockturtle::has_rank_position_v<Ntk>, "Ntk does not implement the rank_position function");🤖 Prompt for AI Agents
In
`@include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp`
around lines 844 - 846, The static_assert messages incorrectly reference
"NtkSrc" instead of the actual template parameter name `Ntk`; update the three
static_assert messages that mention mockturtle::is_network_type_v<Ntk>,
mockturtle::has_create_node_v<Ntk>, and mockturtle::has_rank_position_v<Ntk> to
refer to "Ntk" (e.g., "Ntk is not a network type", "Ntk does not implement the
create_node function", "Ntk does not implement the rank_position function") so
the error text matches the template parameter name used in
node_duplication_planarization.hpp.
There was a problem hiding this comment.
Pull request overview
This PR introduces a planarization framework for logic networks and a planar physical design flow for Field-Coupled Nanocomputing (FCN) circuits. It implements node duplication planarization based on the paper "Fabricatable Interconnect and Molecular QCA Circuits" and the Planar Layout from Network Embedding (PLANE) algorithm for generating crossing-free layouts.
Changes:
- Node duplication planarization algorithm that solves the Node Duplication Crossing Minimization (NDCE) problem using H-graph shortest path finding
- PLANE algorithm for generating planar 2DDWave-clocked Cartesian gate-level layouts from planar network embeddings
- CLI command
planethat performs the full physical design flow (fanout substitution, balancing, planarization, and layout generation)
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
include/fiction/algorithms/network_transformation/node_duplication_planarization.hpp |
Implements node duplication planarization using H-graph for crossing minimization |
include/fiction/algorithms/physical_design/planar_layout_from_network_embedding.hpp |
Implements PLANE algorithm for planar layout generation from network embeddings |
test/algorithms/network_transformation/node_duplication_planarization.cpp |
Comprehensive tests for planarization including equivalence checking and crossing verification |
test/algorithms/physical_design/planar_layout_from_network_embedding.cpp |
Tests for PLANE algorithm including name conservation and exception handling |
cli/cmd/physical_design/src/plane.cpp |
CLI command implementation for the PLANE flow |
cli/cmd/physical_design/include/plane.hpp |
Header for the PLANE CLI command |
cli/cmd/physical_design/cmd_physical_design.hpp |
Registration of the PLANE command |
docs/changelog.rst |
Changelog entry documenting the new algorithms |
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp |
Auto-generated Python binding documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Network type from which node types are drawn.)doc"; | ||
|
|
||
| static const char *__doc_fiction_detail_hgraph_node_delay = R"doc(Specifies the delay value for the hgraph_node.)doc"; | ||
|
|
There was a problem hiding this comment.
Missing documentation in generated docstrings: The "middle_fanins" member of hgraph_node is missing from the pybind11 docstrings. Since "outer_fanins", "delay", and "fanin_it" are documented (lines 8917-8919), "middle_fanins" should also be included.
| static const char *__doc_fiction_detail_hgraph_node_middle_fanins = R"doc(Stores the fanins between the first and last positions of the ordering. | |
| Their mutual order is irrelevant for the H-graph algorithm.)doc"; |
| */ | ||
| std::vector<std::vector<hgraph_node<Ntk>>> lvl_pairs{}; | ||
| /** | ||
| * The fanin nodes. |
There was a problem hiding this comment.
Unclear member documentation: The "fis" member (line 810) is documented as "The fanin nodes." which is ambiguous. It should be more descriptive, such as "Temporary storage for fanin nodes during slice delay computation." or similar based on its usage in the code.
| * The fanin nodes. | |
| * Temporary storage for the fanin nodes of the currently processed node | |
| * during the node duplication planarization procedure. |
| * Variant of the `mockturtle::initialize_copy_network`. This function helps with creating new networks from old | ||
| * networks. In the mockturtle/original version `old2new` is used to map nodes from the old network to nodes in the new | ||
| * network in a one-to-one relation. This variant allows old nodes to map to multiple nodes in order to represent | ||
| * relations to dulicated nodes. |
There was a problem hiding this comment.
Typo in comment: "dulicated" should be "duplicated"
| * relations to dulicated nodes. | |
| * relations to duplicated nodes. |
| 0 : | ||
| propagate_left + cluster_new_lines[i] - two_input_new_lines[i - 1]; | ||
|
|
||
| // ALso set the new lines for two input nodes |
There was a problem hiding this comment.
Typo in comment: "ALso" should be "Also"
| // ALso set the new lines for two input nodes | |
| // Also set the new lines for two input nodes |
| // Resolve new lines. Special case for the second fan-out of a fan-out node | ||
| if (ntk.is_fanout(pre) && (orientation[i] == 2 || orientation[i] == 3)) | ||
| { | ||
| // n the special case the |
There was a problem hiding this comment.
Incomplete comment: "n the special case the" appears to be missing a word at the beginning. Should likely be "In the special case the"
| // n the special case the | |
| // In the special case the |
| { | ||
| std::sort(fc.fanin_nodes.begin(), fc.fanin_nodes.end(), [&ntk](int a, int b) | ||
| { return ntk.rank_position(a) < ntk.rank_position(b); }); | ||
| // compute the max_gap for two fan-ins of anode |
There was a problem hiding this comment.
Typo in documentation: "anode" should be "a node" in the comment. There's also a double space before "anode".
| // compute the max_gap for two fan-ins of anode | |
| // compute the max_gap for two fan-ins of a node |
| /** | ||
| * Checks if the given vector of nodes contains any non-primary inputs. | ||
| * | ||
| * @param v_next_level The vector of nodes to be checked. |
There was a problem hiding this comment.
Missing return documentation: The check_final_level function (line 692) has a description but is missing documentation for the return value. It should document that it returns true if all nodes in the vector are primary inputs, false otherwise.
| * @param v_next_level The vector of nodes to be checked. | |
| * @param v_next_level The vector of nodes to be checked. | |
| * @return `true` if all nodes in `v_next_level` are primary inputs, `false` otherwise. |
|
|
||
| // Check if the candidate matches the original fan-in or a duplicate. | ||
| // Also, verify if the candidate has already reached its fan-out limit. | ||
| // THis adjusts for virtual PIS |
There was a problem hiding this comment.
Typo in comment: "THis" should be "This"
| // THis adjusts for virtual PIS | |
| // This adjusts for virtual PIS |
| * This function computes the order of nodes in the next level based on their delay in the H-graph of the level. It | ||
| * selects the path with the least delay from the current level pairs and follows it via fanin relations. The nodes | ||
| * are inserted into the next level vector in the order they are encountered. | ||
| * | ||
| * @return The order of nodes in `next_level` |
There was a problem hiding this comment.
Incomplete function documentation: The compute_node_order function (line 624) has a brief description but is missing important details such as what it returns and how it uses the H-graph data structure. The return documentation only says "The order of nodes in next_level" but should be more specific about what this ordering represents.
| * This function computes the order of nodes in the next level based on their delay in the H-graph of the level. It | |
| * selects the path with the least delay from the current level pairs and follows it via fanin relations. The nodes | |
| * are inserted into the next level vector in the order they are encountered. | |
| * | |
| * @return The order of nodes in `next_level` | |
| * Computes a processing order for the nodes of the next network level from the level H-graph. | |
| * | |
| * The level H-graph is stored in the last entry of `lvl_pairs` as a collection of `hgraph_node<Ntk>` objects, | |
| * each of which represents a possible path (or partial path) through the current level together with its | |
| * accumulated delay. This function selects the H-graph node with the minimal delay and follows this path | |
| * backwards via fanin relations in the underlying network, inserting the encountered nodes into | |
| * `next_level` using `insert_if_not_first` to respect fanout saturation constraints. | |
| * | |
| * The resulting vector therefore encodes the sequence in which nodes in the *next* level should be considered | |
| * by the node duplication planarization algorithm: it is an ordered list of `mockturtle::node<Ntk>` objects | |
| * along the chosen minimum-delay path in the H-graph, with earlier elements expected to be processed (or | |
| * placed) before later ones. | |
| * | |
| * @return A vector containing the nodes of the next level in the order implied by the minimum-delay H-graph path. |
| struct hgraph_node | ||
| { | ||
| /** | ||
| * First and last fanin. |
There was a problem hiding this comment.
Missing documentation for member variable: The "outer_fanins" member of the hgraph_node struct (line 77) is documented with just "First and last fanin." which should be more descriptive, such as "Pair containing the first and last fanins in the ordering."
| * First and last fanin. | |
| * Pair containing the first (leftmost) and last (rightmost) fanins in the ordering. |
Description
This PR includes the files for planarization of networks via node duplication and for the planar placement and routing.
Not: It also contains files for check_planarity, which checks if a given planar embedding has no crossings. This could also be checked using mincross from PR #736.
Fixes # (issue)
Checklist:
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.