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>
…functionalities inside, which arenow in the molQCA.cpp
Signed-off-by: GitHub Actions <actions@github.com>
Signed-off-by: GitHub Actions <actions@github.com>
WalkthroughAdds MolQCA support across types, I/O, SVG rendering, a molecular gate library and tests; converts several public enums to scoped enum classes; introduces MolQCA-aware traits and types; enriches Python bindings with extensive docstrings and updates tests and CLI integration. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI cell command
participant Lib as molecular_qca_library
participant Layout as Cell layout store
participant Writer as write_mol_qca_layout_svg_impl
participant FS as filesystem / ostream
CLI->>Lib: apply_gate_library(layout, molecular_qca_library)
alt success
Lib-->>CLI: return cell layout with MolQCA gates
CLI->>Layout: update stored layout
CLI->>Writer: write_mol_qca_layout_svg(layout, params)
Writer->>Writer: select renderer (cell/tile), compute phases & viewbox
Writer->>FS: emit SVG elements (cells, vias, clock zones)
FS-->>Writer: write OK
else failure
Lib-->>CLI: error/exception
CLI-->>CLI: show error message
end
sequenceDiagram
autonumber
participant User as API user
participant API as write_mol_qca_layout_svg(...)
participant Impl as write_mol_qca_layout_svg_impl
participant Stream as ostream
User->>API: call with layout + params
API->>Impl: select rendering path (cell-based / tile-based)
Impl->>Impl: compute colors, descriptions, viewbox
Impl->>Stream: write SVG chunks
Stream-->>User: completed SVG output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (7)
include/fiction/layouts/gate_level_layout.hpp (1)
701-708: Make helper noexcept and guard index; consider encapsulation
revive_from_deadis a thin guard aroundrevive_node. Mark itnoexcept, add a debug bound check, and consider moving the call into the sites that require it (comment already hints it “should not exist”). Keeps API surface smaller.Apply:
- void revive_from_dead(const node n) + void revive_from_dead(const node n) noexcept { - if (!is_constant(n) && is_dead(n)) + assert(n < size()); + if (!is_constant(n) && is_dead(n)) { revive_node(n); } }test/technology/qca_one_library.cpp (1)
8-8: Drop unused include or switch to angle brackets
"fiction/utils/debug/network_writer.hpp"is not used in this test and adds compile overhead. Remove it; if needed later, prefer<fiction/utils/debug/network_writer.hpp>for consistency.include/fiction/technology/cell_technologies.hpp (1)
216-220: Normalize semantics: includeNORMALinis_normal_cellor remove it
mol_qca_technologydefinescell_type::NORMAL = 'x'butis_normal_cellonly returns true forNORMAL1..4. If any MolQCA logic ever emits'x', downstream (e.g., writers) will treat it as unsupported. Either:
- Include
NORMALin the predicate, or- Remove
NORMALfrom the enum to avoid accidental use.Apply:
- [[nodiscard]] static constexpr bool is_normal_cell(const cell_type& c) noexcept - { - return (c == NORMAL1 || c == NORMAL2 || c == NORMAL3 || c == NORMAL4); - } + [[nodiscard]] static constexpr bool is_normal_cell(const cell_type& c) noexcept + { + return (c == NORMAL || c == NORMAL1 || c == NORMAL2 || c == NORMAL3 || c == NORMAL4); + }Optionally, add a brief comment explaining that MolQCA prefers
a..dbut acceptsxfor compatibility.test/technology/molecular_qca_library.cpp (1)
31-90: Reduce duplication for gate templatesThe large
primary_input_port/primary_output_port/disjunctionblocks repeat across tests. Consider a small helper/factory to keep tests terse and easier to update.experiments/molQCA/molQCA.cpp (3)
2-2: Duplicate include of exact.hppBoth quoted and angle-bracket includes pull in the same header. Keep one.
-#include "fiction/algorithms/physical_design/exact.hpp" #include "fiction/algorithms/physical_design/exact.hpp" @@ -#include <fiction/algorithms/physical_design/exact.hpp>Also applies to: 11-11
438-446: Prefer node type for map key instead of raw integerUse mockturtle::node as the key to avoid implicit conversions and improve type safety.
- std::unordered_map<uint64_t, std::vector<mockturtle::signal<Lyt>>> fanin_relations; + std::unordered_map<mockturtle::node<Lyt>, std::vector<mockturtle::signal<Lyt>>> fanin_relations; @@ - fanin_relations[n] = fanins; + fanin_relations[n] = std::move(fanins);
603-611: Loop ignores selected benchmarks and hardcodes half_adderEither use the benchmark from the loop or remove the loop to avoid repeating the same run.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(6 hunks)experiments/molQCA/molQCA.cpp(1 hunks)include/fiction/io/write_qll_layout.hpp(6 hunks)include/fiction/io/write_svg_layout.hpp(4 hunks)include/fiction/layouts/gate_level_layout.hpp(1 hunks)include/fiction/technology/cell_technologies.hpp(1 hunks)include/fiction/technology/molecular_qca_library.hpp(1 hunks)include/fiction/traits.hpp(1 hunks)include/fiction/types.hpp(1 hunks)test/technology/molecular_qca_library.cpp(1 hunks)test/technology/qca_one_library.cpp(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
test/technology/molecular_qca_library.cpp (1)
include/fiction/technology/molecular_qca_library.hpp (1)
set_up_gate(39-39)
include/fiction/technology/molecular_qca_library.hpp (4)
include/fiction/io/write_svg_layout.hpp (8)
nodiscard(107-110)nodiscard(644-648)nodiscard(659-700)lyt(842-852)c(889-966)c(889-889)c(1260-1345)c(1260-1260)include/fiction/layouts/gate_level_layout.hpp (34)
nodiscard(218-225)nodiscard(231-235)nodiscard(237-240)nodiscard(242-245)nodiscard(275-278)nodiscard(279-282)nodiscard(289-292)nodiscard(294-298)t(289-289)t(310-310)t(658-658)t(783-831)t(783-783)t(974-974)t(984-984)t(994-994)t(1707-1738)t(1707-1707)n(237-237)n(242-242)n(275-275)n(279-279)n(294-294)n(300-300)n(346-349)n(346-346)n(356-356)n(371-371)n(512-512)n(669-669)n(686-686)n(697-697)n(702-708)n(702-702)include/fiction/technology/fcn_gate_library.hpp (1)
unsupported_gate_orientation_exception(56-60)include/fiction/technology/cell_technologies.hpp (10)
c(68-68)c(73-73)c(78-78)c(83-83)c(88-88)c(93-93)c(98-98)c(211-211)c(216-216)c(221-221)
include/fiction/io/write_qll_layout.hpp (1)
include/fiction/io/write_svg_layout.hpp (5)
lyt(842-852)c(889-966)c(889-889)c(1260-1345)c(1260-1260)
include/fiction/technology/cell_technologies.hpp (2)
include/fiction/io/write_svg_layout.hpp (7)
nodiscard(107-110)nodiscard(644-648)nodiscard(659-700)c(889-966)c(889-889)c(1260-1345)c(1260-1260)include/fiction/layouts/cell_level_layout.hpp (16)
nodiscard(142-148)nodiscard(192-200)nodiscard(208-224)nodiscard(231-234)nodiscard(241-244)nodiscard(269-277)c(162-185)c(162-162)c(192-192)c(241-241)c(252-262)c(252-252)c(269-269)c(285-295)c(285-285)c(302-302)
experiments/molQCA/molQCA.cpp (3)
include/fiction/layouts/gate_level_layout.hpp (30)
name(247-257)name(247-247)name(336-339)name(336-336)n(237-237)n(242-242)n(275-275)n(279-279)n(294-294)n(300-300)n(346-349)n(346-346)n(356-356)n(371-371)n(512-512)n(669-669)n(686-686)n(697-697)n(702-708)n(702-702)t(289-289)t(310-310)t(658-658)t(783-831)t(783-783)t(974-974)t(984-984)t(994-994)t(1707-1738)t(1707-1707)include/fiction/io/write_qll_layout.hpp (8)
os(210-213)os(215-238)os(240-261)pi(263-273)write_qll_layout(437-446)write_qll_layout(437-437)write_qll_layout(460-471)write_qll_layout(460-460)include/fiction/io/write_svg_layout.hpp (10)
x(644-645)x(659-660)write_qca_layout_svg(1608-1617)write_qca_layout_svg(1608-1608)write_qca_layout_svg(1634-1645)write_qca_layout_svg(1634-1634)write_mol_qca_layout_svg(1662-1671)write_mol_qca_layout_svg(1662-1662)write_mol_qca_layout_svg(1685-1697)write_mol_qca_layout_svg(1685-1686)
include/fiction/io/write_svg_layout.hpp (3)
include/fiction/io/write_qll_layout.hpp (5)
c(170-170)c(175-175)os(210-213)os(215-238)os(240-261)include/fiction/io/read_fqca_layout.hpp (4)
c(253-256)c(253-253)c(258-325)c(258-258)include/fiction/io/write_qca_layout.hpp (1)
lyt(198-232)
🔇 Additional comments (1)
include/fiction/traits.hpp (1)
585-586: New molQCA trait looks correctThe variable template mirrors existing technology checks and uses the same pattern. No concerns.
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
10878-10906: Fill missing writer-impl docstrings; avoid exposing “unused” tile-based variant.Several molQCA SVG writer implementation docstrings are empty; the tile-based variant is marked “Unused.” Provide brief, accurate texts and mark the tile-based path as internal to avoid confusing users.
-static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl = R"doc()doc"; +static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl = +R"doc(Implementation of the molQCA SVG writer for cell-based layouts.)doc"; @@ -R"doc(Generates an SVG string representing the cell-based clocked cell -layout and appends it to the output stream.)doc"; +R"doc(Generates an SVG string for a cell-based molQCA layout and appends it +to the output stream.)doc"; @@ -R"doc(Generates an SVG string representing the tile-based clocked cell -layout and appends it to the output stream. Unused.)doc"; +R"doc(Internal: Generates an SVG string for a tile-based molQCA layout +composed of :math:`10 \times 10` cells; currently not used by the Python +bindings.)doc"; @@ -static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_lyt = R"doc()doc"; +static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_lyt = +R"doc(Cell-level molQCA layout.)doc"; @@ -static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_os = R"doc()doc"; +static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_os = +R"doc(Output stream to receive the SVG.)doc"; @@ -static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_ps = R"doc()doc"; +static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_ps = +R"doc(SVG writer parameters controlling colors, sizes, and scaling.)doc"; @@ -static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_run = R"doc()doc"; +static const char *__doc_fiction_detail_write_mol_qca_layout_svg_impl_run = +R"doc(Runs the writer on the given layout and parameters.)doc";
16681-16706: Tighten molQCA technology/library docs and remove “QCA” remnants.Replace generic/empty texts and “QCA” remnants with concise molQCA wording; adjust one misleading “QCA ONE gate”.
-static const char *__doc_fiction_mol_qca_technology = R"doc()doc"; +static const char *__doc_fiction_mol_qca_technology = +R"doc(Technology definition for molQCA layouts: enums, marks, modes, and types.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_mode = R"doc(Possible cell modes for QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_mode = +R"doc(Possible cell modes for molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_CONST_0 = R"doc(Symbol used for constant 0 input QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_CONST_0 = +R"doc(Symbol used for constant 0 input molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_CONST_1 = R"doc(Symbol used for constant 1 input QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_CONST_1 = +R"doc(Symbol used for constant 1 input molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_EMPTY = R"doc(Symbol used for empty QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_EMPTY = +R"doc(Symbol used for empty molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_INPUT = R"doc(Symbol used for input QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_INPUT = +R"doc(Symbol used for input molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL = R"doc(Symbol used for normal QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL = +R"doc(Symbol used for normal molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL1 = R"doc(Symbol used for normal QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL1 = +R"doc(Symbol used for normal molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL2 = R"doc(Symbol used for normal QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL2 = +R"doc(Symbol used for normal molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL3 = R"doc(Symbol used for normal QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL3 = +R"doc(Symbol used for normal molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL4 = R"doc(Symbol used for normal QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_NORMAL4 = +R"doc(Symbol used for normal molQCA cells.)doc"; @@ -static const char *__doc_fiction_mol_qca_technology_cell_type_OUTPUT = R"doc(Symbol used for output QCA cells.)doc"; +static const char *__doc_fiction_mol_qca_technology_cell_type_OUTPUT = +R"doc(Symbol used for output molQCA cells.)doc"; @@ -static const char *__doc_fiction_molecular_qca_library_determine_port_routing = R"doc()doc"; +static const char *__doc_fiction_molecular_qca_library_determine_port_routing = +R"doc(Determines routing between gate ports inside a tile, honoring molQCA +constraints and available vias.)doc"; @@ -static const char *__doc_fiction_molecular_qca_library_molecular_qca_library = R"doc()doc"; +static const char *__doc_fiction_molecular_qca_library_molecular_qca_library = +R"doc(Constructs the Politecnico di Torino molQCA gate library from +SCERPA‑validated templates.)doc"; @@ -Parameter ``t``: - Tile to be realized as a QCA ONE gate. +Parameter ``t``: + Tile to be realized as a molQCA gate.Optional: many remaining entries are still empty (e.g., marks, predicates). If you want, I can batch‑fill them to keep Python help() useful.
Also applies to: 16774-16792
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T10:31:01.287Z
Learnt from: marcelwa
PR: cda-tum/fiction#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
🧬 Code graph analysis (1)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
bindings/mnt/pyfiction/include/pyfiction/technology/area.hpp (2)
area(16-52)area(19-43)
🔇 Additional comments (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
22946-22968: Fix molQCA SVG stream overload docstring to match the file overload (10×10, cell-based).The stream overload still documents legacy QCA 5×5 tiles and mixed tile-/cell-based support. Align it with the file overload below (molQCA, cell-based, 10×10).
static const char *__doc_fiction_write_mol_qca_layout_svg = -R"doc(Writes an SVG representation of a cell-level QCA layout into an output -stream. Both tile- and cell-based layouts are supported. For tile- -based layouts, QCA layouts of tile size :math:`5 \times 5` are -supported exclusively so far. - -The utilized color scheme is based on the standard scheme used in -QCADesigner (https://waluslab.ece.ubc.ca/qcadesigner/). - -May throw an `unsupported_cell_type_exception` if it encounters -unsupported cell types in the layout. - -Template parameter ``Lyt``: - Cell-level QCA layout type. - -Parameter ``lyt``: - The layout to be written. - -Parameter ``os``: - The output stream to write into. - -Parameter ``ps``: - Parameters.)doc"; +R"doc(Writes an SVG representation of a cell-level molQCA layout into an output +stream. Cell-based layouts composed of uniform :math:`10 \times 10` +molQCA gates are supported. + +The utilized color scheme is similar to the default in +QCADesigner (https://waluslab.ece.ubc.ca/qcadesigner/). + +May throw an `unsupported_cell_type_exception` if it encounters +unsupported cell types in the layout. + +Template parameter ``Lyt``: + Cell-level molQCA layout type. + +Parameter ``lyt``: + The layout to be written. + +Parameter ``os``: + The output stream to write into. + +Parameter ``ps``: + SVG writer parameters (colors, sizes, scaling).)doc";
10878-10883: No action needed: pyfiction headers remain private and are not installed.CMakeLists.txt marks the bindings include directory as PRIVATE (
target_include_directories(pyfiction PRIVATE ...)) and only installs the pyfiction target—not any headers.
# Conflicts: # cli/cmd/technology/cell.hpp # cli/stores.hpp
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (3)
23291-23313: Fix molQCA SVG docstring mismatch (see existing comment).The stream overload docstring still describes the legacy QCA writer with 5×5 tile support, contradicting the file overload below that correctly documents molQCA with 10×10 tiles. This inconsistency will mislead Python binding users.
23583-23587: Unify terminology: prefer "molQCA (mQCA)" (see existing comment).The docstring uses "mQCA technology platform" but should use "molQCA (mQCA) technology platform" for consistency with the rest of the API.
23604-23608: Unify terminology: prefer "molQCA (mQCA)" (see existing comment).Same issue as the stream overload above—use "molQCA (mQCA) technology platform" instead of "mQCA technology platform" for consistency.
cli/stores.hpp (1)
467-477: Error message still omits MolQCA.The guard condition includes MolQCA, but the error message on Line 470 does not mention it.
- cmd.env->out() << fmt::format("[e] {} is neither a QCA layout nor an SiDB layout\n", + cmd.env->out() << fmt::format("[e] {} is neither a QCA, MolQCA, nor an SiDB layout\n", lyt_ptr->get_layout_name());include/fiction/io/write_svg_layout.hpp (1)
1661-1674: Update docstring to reflect molQCA specifics.The docstring still contains QCA-specific references that need adjustment:
- Line 1662: Says "QCA layout" but should say "molQCA layout"
- Line 1665: References QCADesigner which is not relevant for molQCA
- Line 1663: Mentions "tile- and cell-based layouts" but MolQCA only supports cell-based (line 1221)
Apply this diff:
/** - * Writes an SVG representation of a cell-level QCA layout into an output stream. Both tile- and cell-based layouts are - * supported. For tile-based layouts, QCA layouts of tile size \f$5 \times 5\f$ are supported exclusively so far. + * Writes an SVG representation of a cell-level molQCA layout into an output stream. Cell-based layouts are + * supported. * - * The utilized color scheme is based on the standard scheme used in QCADesigner - * (https://waluslab.ece.ubc.ca/qcadesigner/). + * The utilized color scheme is specific to molQCA technology. * * May throw an `unsupported_cell_type_exception` if it encounters unsupported cell types in the layout. * - * @tparam Lyt Cell-level QCA layout type. + * @tparam Lyt Cell-level molQCA layout type. * @param lyt The layout to be written. * @param os The output stream to write into. * @param ps Parameters. */
🧹 Nitpick comments (4)
cli/stores.hpp (1)
486-489: Consider supporting the--simpleflag for MolQCA.The MolQCA rendering path passes empty parameters
{}, ignoring the--simpleflag that QCA honors on Line 484. This creates an inconsistent user experience.Apply this diff if MolQCA should support simplified rendering:
- fiction::write_mol_qca_layout_svg(*lyt_ptr, os, {}); + fiction::write_mol_qca_layout_svg(*lyt_ptr, os, {cmd.is_set("simple")});If the simple flag is intentionally unsupported for MolQCA, consider adding a comment explaining why.
include/fiction/io/write_svg_layout.hpp (3)
448-448: Standardize spelling of MolQCA.The comment uses "MOL QCA" while constants use "MOL_QCA" and identifiers use "MolQCA"/"molQCA". Consider standardizing the spelling across comments, identifiers, and documentation.
551-561: Address or track the TODO comment.The TODO comment indicates incomplete work on the
MOL_QCA_SIMPLE_CELLtemplate. Please clarify what adjustment is needed or remove the comment if the current implementation is sufficient.Would you like me to open an issue to track this TODO, or can you clarify what adjustment is needed?
1412-1602: Remove or document the unused tile-based rendering.The
generate_tile_based_svg()function is marked "Unused" and never called. Dead code increases maintenance burden and can cause confusion.Consider either:
- Remove the function if tile-based rendering is not planned for MolQCA
- If kept for future use, add a clear comment explaining why and when it will be used
/** * Generates an SVG string representing the tile-based clocked cell layout and appends it to the output stream. - * Unused. + * Reserved for future tile-based MolQCA layout support. Currently, MolQCA only uses cell-based rendering. */Or remove entirely if not needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(5 hunks)cli/cmd/technology/src/cell.cpp(3 hunks)cli/stores.hpp(2 hunks)include/fiction/io/write_svg_layout.hpp(6 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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:
cli/stores.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/io/write_svg_layout.hpp
🧬 Code graph analysis (2)
cli/stores.hpp (2)
include/fiction/io/write_svg_layout.hpp (4)
write_mol_qca_layout_svg(1676-1685)write_mol_qca_layout_svg(1676-1676)write_mol_qca_layout_svg(1699-1711)write_mol_qca_layout_svg(1699-1700)include/fiction/io/write_qll_layout.hpp (3)
os(219-222)os(224-243)os(245-262)
include/fiction/io/write_svg_layout.hpp (3)
include/fiction/layouts/coordinates.hpp (11)
c(1031-1034)c(1031-1031)c(1041-1044)c(1041-1041)c(1051-1054)c(1051-1051)static_cast(133-136)os(295-299)os(295-295)coord(964-978)coord(989-992)include/fiction/io/write_qca_layout.hpp (1)
lyt(198-232)include/fiction/layouts/cartesian_layout.hpp (4)
coord(108-111)coord(108-108)coord(821-829)coord(821-821)
⏰ 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). (18)
- GitHub Check: 🚨 Clang-Tidy
- GitHub Check: 🐍 ubuntu-latest
- GitHub Check: 🐍 windows-latest
- GitHub Check: 🛞 Wheels for windows-latest
- GitHub Check: 🍎 macos-13 with g++-14
- GitHub Check: 🍎 macos-13 with g++-13
- GitHub Check: 🍎 macos-14 with clang++
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-17
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-18
- GitHub Check: 🐧 ubuntu-24.04 with clang++-16
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-16
- GitHub Check: 🐧 ubuntu-24.04 with g++-11
- GitHub Check: 🐧 ubuntu-24.04 with g++-14
- GitHub Check: 🐧 ubuntu-22.04 with g++-11
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-15
- GitHub Check: 🐧 ubuntu-22.04 with clang++-14
- GitHub Check: Coverage on ubuntu-24.04 with g++-13
- GitHub Check: 🐳 Build and publish Docker image
🔇 Additional comments (6)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
10980-11010: LGTM: Internal implementation docstrings.The docstrings for the
write_mol_qca_layout_svg_impldetail class are appropriately brief. Empty docstrings for private members and concise descriptions for internal methods are acceptable for implementation details.
17011-17123: LGTM: Comprehensive molQCA technology docstrings.The docstrings for
mol_qca_technologyandmolecular_qca_libraryare thorough, well-structured, and use consistent terminology. The technical details (10×10 tile size) and API documentation are clear and helpful for Python binding users.cli/cmd/technology/src/cell.cpp (1)
7-7: LGTM!Include added to support MolQCA library integration.
include/fiction/io/write_svg_layout.hpp (3)
1286-1308: LGTM!The MolQCA implementation determines clock zones from cell type variants (normal_cell1-4) rather than using
get_clock_number()as QCA does. This reflects a fundamental difference in how MolQCA technology encodes clock information. The bounds-checked.at(idx)access is appropriate.
1319-1323: LGTM!Throwing
unsupported_cell_type_exceptionfor crossings is appropriate since MolQCA doesn't support this feature yet. This provides a clear error message rather than incorrect rendering.Consider verifying whether this limitation is documented in user-facing documentation or tracked as a future enhancement.
1650-1650: LGTM!Converting
filenametostd::stringbefore passing tostd::ofstreamconstructor is safer than using.data()as it ensures proper null-termination and string handling.Also applies to: 1702-1702, 1752-1752
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/algorithms/simulation/sidb/random_sidb_layout_generator.cpp (1)
148-173: Cube and offset layouts have invertedpositive_chargesenum values; SiQAD and defect sections do notThe review correctly identifies real issues in the cube and offset test layouts (lines 148-173, 175-202, 408-433, 435-462), where section titles contradict their enum arguments:
- "forbid positive charges" sections use
ALLOWED(should beFORBIDDEN)- "allow positive charges" sections use
FORBIDDEN(should beALLOWED)However, the SiQAD and defect sections are not affected by this issue. Lines 685-708 and 849-886 correctly pair "allow" titles with
ALLOWEDenums, and lines 710-734 correctly pair "forbid" titles withFORBIDDENenums.The additional concern about missing assertions on the
ALLOWEDpath holds only for cube/offset layouts, where neither "forbid" (ALLOWED) nor "allow" (FORBIDDEN) section validatescan_positive_charges_occuron an actual ALLOWED configuration.Recommended action: Fix the inverted enum values in cube and offset test sections (lines 148-173, 175-202, 408-433, 435-462) to align section titles with semantics, then add assertions to verify both ALLOWED and FORBIDDEN paths exercise the intended behavior.
include/fiction/types.hpp (1)
280-281: Likely alias bug:cds_sidb_cell_clk_lyt_cubeuses the SiQAD layout type.This pre-existing alias looks inconsistent:
using cds_sidb_cell_clk_lyt_cube = charge_distribution_surface<sidb_cell_clk_lyt_siqad>;Given the name, it probably should wrap
sidb_cell_clk_lyt_cuberather than the SiQAD layout:-using cds_sidb_cell_clk_lyt_cube = charge_distribution_surface<sidb_cell_clk_lyt_siqad>; +using cds_sidb_cell_clk_lyt_cube = charge_distribution_surface<sidb_cell_clk_lyt_cube>;Otherwise, both the
*_siqadand*_cubecharge-distribution aliases refer to the same underlying layout type, which is confusing and could lead to subtle misuse downstream.
♻️ Duplicate comments (4)
include/fiction/io/write_qll_layout.hpp (1)
381-427: Use the layout’s clock number for MolQCAPROPERTY_PHASE(normal + via cells)
idxis derived purely from the MolQCA cell type. For vias (in particular the ground segments materialised during post‑layout optimisation) the type remains a genericNORMAL, soidxalways collapses to0. As a result, all vertically repaired wires are exported with phase 0, desynchronising the exported QLL clocking from the actual layout clocks (similar to the earlier issue fixed for QCA).To preserve the correct clocking for both regular and via MolQCA cells, use the layout’s recorded clock number instead of the type‑derived
idxwhen writingPROPERTY_PHASE:- const int idx = [&type] - { - if (Lyt::technology::is_normal_cell1(type)) - { - return 0; - } - if (Lyt::technology::is_normal_cell2(type)) - { - return 1; - } - if (Lyt::technology::is_normal_cell3(type)) - { - return 2; - } - if (Lyt::technology::is_normal_cell4(type)) - { - return 3; - } - return 0; - }(); + // Use the layout's clock number to encode the phase in QLL. @@ - if (mol_qca_technology::is_normal_cell(type)) + if (mol_qca_technology::is_normal_cell(type)) { os << fmt::format(qll::OPEN_MQCA_LAYOUT_ITEM, 0, cell_id++, bb_x(c), bb_y(c), c.z * 2); - os << fmt::format(qll::LAYOUT_ITEM_PROPERTY, qll::PROPERTY_PHASE, idx); + os << fmt::format(qll::LAYOUT_ITEM_PROPERTY, qll::PROPERTY_PHASE, + lyt.get_clock_number(c)); os << qll::CLOSE_LAYOUT_ITEM; } @@ - if (mol_qca_technology::is_vertical_cell_mode(mode) && c.z != lyt.z()) + if (mol_qca_technology::is_vertical_cell_mode(mode) && c.z != lyt.z()) { os << fmt::format(qll::OPEN_MQCA_LAYOUT_ITEM, 0, cell_id++, bb_x(c), bb_y(c), (c.z * 2) + 1); - os << fmt::format(qll::LAYOUT_ITEM_PROPERTY, qll::PROPERTY_PHASE, idx); + os << fmt::format(qll::LAYOUT_ITEM_PROPERTY, qll::PROPERTY_PHASE, + lyt.get_clock_number(c)); os << qll::CLOSE_LAYOUT_ITEM; }You can drop
idxentirely if it’s no longer used elsewhere.bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
23576-23581: Unify terminology: prefer “molQCA (mQCA) technology platform” in QLL docstrings.Both QLL writer docstrings still end with “mQCA technology platform”. For consistency with the rest of the API and to introduce the acronym, consider:
R"doc(Writes a cell-level QCA, molQCA or iNML layout to a qll file that is used by ToPoliNano & MagCAD (https://topolinano.polito.it/), an EDA tool and a physical simulator for the iNML technology platform as well -as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical -simulator for the mQCA technology platform. +as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical +simulator for the molQCA (mQCA) technology platform.Apply the same change to both overloads.
Also applies to: 23597-23601
include/fiction/io/write_svg_layout.hpp (1)
1661-1683: Docstring still mentions “QCA” in MolQCA API template parameter.In the MolQCA stream overload, the
@tparamline still says “Cell-level QCA layout type”, while the rest of the comment and function clearly target MolQCA.Consider updating it to:
- * @tparam Lyt Cell-level QCA layout type. + * @tparam Lyt Cell-level molQCA layout type.to keep the documentation consistent with the new technology.
include/fiction/types.hpp (1)
178-201: Missingmol_qca_technologybranch intech_cell_name.
tech_cell_namecurrently distinguishes QCA, iNML, and SiDB, but there is no branch formol_qca_technology, sotech_cell_name<mol_qca_technology>will yield"?"instead of a meaningful cell label.To keep MolQCA on par with QCA, consider:
-constexpr const char* qca_cell_name = "cells"; -constexpr const char* inml_cell_name = "magnets"; -constexpr const char* sidb_cell_name = "dots"; +constexpr const char* qca_cell_name = "cells"; +constexpr const char* mol_qca_cell_name = "cells"; +constexpr const char* inml_cell_name = "magnets"; +constexpr const char* sidb_cell_name = "dots"; template <class Tech> inline constexpr const char* tech_cell_name = [] { if constexpr (std::is_same_v<std::decay_t<Tech>, qca_technology>) { return qca_cell_name; } + else if constexpr (std::is_same_v<std::decay_t<Tech>, mol_qca_technology>) + { + return mol_qca_cell_name; + } else if constexpr (std::is_same_v<std::decay_t<Tech>, inml_technology>) { return inml_cell_name; } else if constexpr (std::is_same_v<std::decay_t<Tech>, sidb_technology>) { return sidb_cell_name; } else { return "?"; } }();This aligns MolQCA with the rest of the tech stack and matches the earlier intent in prior review feedback.
🧹 Nitpick comments (6)
include/fiction/io/write_qll_layout.hpp (1)
443-460: Update Doxygen@tparam/@paramdocs to reflect MolQCA and current signaturesThe top‑level comments now mention QCA, molQCA, and iNML correctly, but the
@tparam Lytand@param pstags are out of date (nopsparameter, andLytis no longer “QCA or iNML only”). Consider aligning these tags with the actual function signatures and supported technologies.include/fiction/technology/molecular_qca_library.hpp (2)
36-45: Clarify docstring: this sets up a MolQCA/SIM(7)-MolPDK gate, not “QCA ONE”The
@param tdescription still refers to “a QCA ONE gate”. To avoid confusion with the existing QCA ONE library, consider rephrasing this as “MolQCA/SIM(7)-MolPDK gate” (or similar) so the documentation matches the class’ purpose.
170-573: Constexpr gate templates and rotation maps appear structurally sound; minor doc nit in MAJORITY_MAPThe large set of constexpr gate patterns and their rotation‑based lookup tables (
WIRE_MAP,INVERTER_MAP,CONJUNCTION_MAP,DISJUNCTION_MAP,FANOUT_MAP,MAJORITY_MAP) follow the same structure as the existing gate libraries; no obvious mismatches stand out on a structural level.One minor doc nit: the comment for
MAJORITY_MAPsays “Maps ports to corresponding fan-out gates” — that should be “majority gates” for clarity.test/algorithms/simulation/sidb/random_sidb_layout_generator.cpp (1)
596-607: Optionally harmonise remaining.value()uses to the new dereference styleIn a few siqad/defect tests you still use
result_lyt.value()/result_lyts.value()insideif (result_lyt.has_value())blocks. This is safe but diverges from the new pattern where you bindconst auto& lyt = *result_lyt;.For consistency and to keep static analysers quiet uniformly, you may want to switch these remaining spots to the same dereference‑to‑reference style.
Also applies to: 620-630, 643-653, 666-678, 694-707, 719-733, 781-795, 833-846, 866-885, 915-934
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
10980-11009: Internal molQCA SVG impl docstrings are fine; optional wording polish only.The new descriptions for
generate_cell_based_svg,generate_description_color, andgenerate_tile_based_svgare clear and consistent with the implementation naming. The empty placeholders for other internal members are acceptable given they are detail-level, but if these ever surface in Python help() output, consider either:
- removing the unused doc constants, or
- adding a one-liner (e.g., “Implementation detail, not part of the public API”).
This is purely optional and does not block the PR.
include/fiction/io/write_svg_layout.hpp (1)
1412-1602: Tile-based MolQCA SVG generator is unused and still uses generic QCA tile colors.
generate_tile_based_svgis marked “Unused” and is never called fromrun(), but it’s a full copy of the QCA tile-based path with MolQCA cell distances and QCA tile/text colors. That’s fine for now, but it’s dead code.If you don’t plan to wire tile-based MolQCA layouts soon, consider either:
- Removing this method until it’s needed, or
- Adding a brief comment that it’s intentionally kept for future tile-based MolQCA support.
This would reduce maintenance overhead and avoid confusion about supported modes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(6 hunks)cli/cmd/technology/src/cell.cpp(3 hunks)include/fiction/io/write_qll_layout.hpp(8 hunks)include/fiction/io/write_svg_layout.hpp(6 hunks)include/fiction/technology/cell_technologies.hpp(8 hunks)include/fiction/technology/molecular_qca_library.hpp(1 hunks)include/fiction/technology/qca_one_library.hpp(1 hunks)include/fiction/types.hpp(4 hunks)test/algorithms/simulation/sidb/random_sidb_layout_generator.cpp(23 hunks)
✅ Files skipped from review due to trivial changes (1)
- include/fiction/technology/qca_one_library.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/cmd/technology/src/cell.cpp
🧰 Additional context used
🧠 Learnings (5)
📚 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-09-08T13:33:29.693Z
Learnt from: Drewniok
Repo: cda-tum/fiction PR: 826
File: experiments/figure_of_merit_analysis/fom_analysis_2_input_1_output.cpp:5-6
Timestamp: 2025-09-08T13:33:29.693Z
Learning: In the fiction codebase, the preprocessor guard pattern `#if (FICTION_ALGLIB_ENABLED)` is preferred over `#if defined(FICTION_ALGLIB_ENABLED) && FICTION_ALGLIB_ENABLED` for ALGLIB feature detection. The current approach is sufficient for their build system configuration.
Applied to files:
include/fiction/io/write_qll_layout.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/io/write_svg_layout.hppinclude/fiction/technology/cell_technologies.hppinclude/fiction/types.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:
include/fiction/technology/molecular_qca_library.hppinclude/fiction/types.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/types.hpp
🧬 Code graph analysis (1)
include/fiction/technology/molecular_qca_library.hpp (2)
include/fiction/technology/qca_one_library.hpp (1)
determine_port_routing(151-151)include/fiction/technology/fcn_gate_library.hpp (1)
unsupported_gate_orientation_exception(56-60)
🔇 Additional comments (13)
include/fiction/io/write_qll_layout.hpp (2)
145-159: Constexprtech_nameand unified MQCA settings/components look goodThe compile‑time
tech_namelambda and the shared MQCA branches for QCA/molQCA inwrite_technology_settings/write_componentsare consistent and keep the file’s technology handling clean; no issues from my side.Also applies to: 224-243
477-488: Filename overload refactor is safe and preferableConstructing
std::ofstreamfrom astd::stringrather than astd::string_viewdata pointer is the safer choice and avoids subtle lifetime/encoding pitfalls; the open‑check and delegation to the stream overload look good.include/fiction/technology/molecular_qca_library.hpp (2)
32-108: Gate dispatch and error handling inset_up_gatelook consistent with existing librariesThe trait‑guarded
is_*checks, lookup via the dedicated port→gate maps, and the exception mapping (unsupported_gate_orientation_exceptionvs.unsupported_gate_type_exception) mirror the establishedfcn_gate_librarypattern and should behave correctly for supported layouts.
111-168: Port routing for 10×10 MolQCA tiles is straightforward and side-consistentThe connector logic (N/E/S/W in/out at fixed positions with a PI/PO fallback for non‑wire/non‑inv tiles) is clear, side‑consistent, and noexcept‑safe. This matches the expected behaviour for rotation‑aware 10×10 tiles.
test/algorithms/simulation/sidb/random_sidb_layout_generator.cpp (1)
19-42: Optional-handling refactor and enum-path updates in tests look correct
- The new
std::optionalinclude and the consistentif (result.has_value()) { const auto& lyt = *result; ... } else { CHECK(false); }pattern eliminate unchecked-optional warnings while keeping the tests readable.- All updated
can_positive_charges_occurcalls now operate on dereferenced layouts rather than optionals, which is the right fix.- The skeleton-layout tests now correctly use
sidb_100_cell_clk_lyt::technology::cell_type::NORMALafter the enum refactor.I don’t see functional issues in these refactors.
Also applies to: 51-67, 76-117, 120-146, 150-173, 177-202, 214-237, 250-267, 279-295, 305-318, 336-353, 361-378, 386-405, 417-432, 444-462, 474-496, 508-525, 531-547, 552-581, 589-591
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
19266-19271: LGTM: qca_one_library post‑layout optimization docstring.The added blank line before the template parameter section improves readability; content remains accurate and consistent.
include/fiction/io/write_svg_layout.hpp (3)
448-562: MolQCA SVG templates and geometry look consistent with existing QCA shapes.The new MOL_QCA_* color constants, offsets, and SVG snippets are structurally aligned with the QCA definitions and appear internally consistent (distance/offset usage matches the later formatting code). No issues from a structural or correctness perspective.
1206-1410: MolQCA cell-based SVG writer matches technology helpers and QCA behavior.
write_mol_qca_layout_svg_implcorrectly reuses the QCA pipeline while:
- Using
is_normal_cell1..4to pick one of four MolQCA clock-zone colors.- Falling back to
MOL_QCA_SIMPLE_CELL/MOL_QCA_VIA/MOL_QCA_CELLconsistent with QCA.- Respecting synchronization elements and using
MOL_QCA_CELL_DISTANCEand MolQCA-specific offsets.This should render MolQCA layouts analogously to QCA without introducing new corner-case bugs.
1648-1658: Good fix: preferstd::string(filename)when openingstd::ofstream.Switching from using the raw
std::string_viewbuffer tostd::string(filename)for the QCA and SiDB file-based writers avoids relying on null-termination of the underlying view and is a safer, more idiomatic pattern.Also applies to: 1751-1758
include/fiction/technology/cell_technologies.hpp (1)
143-300: MolQCA technology definition aligns with SVG writer expectations.The new
mol_qca_technology:
- Introduces
cell_typewithNORMAL1..4, I/O, and constants.- Provides
is_normal_cell1..4plusis_normal_cellthat aggregates all normal variants.- Supplies the same mode helpers (
is_vertical_cell_mode, etc.) asqca_technology.This matches how
write_mol_qca_layout_svg_implqueries the technology and should integrate cleanly with existing traits and layout code. The enum-class conversions for QCA, iNML, and SiDB also look consistent and non-breaking.include/fiction/types.hpp (3)
76-101: constexpr lambda-basedntk_type_nameis clear and type-safe.The new
ntk_type_nameimplementation using a constexpr lambda andstd::decay_tcleanly handles all supported network types and falls back to"?"otherwise. This keeps compile-time name resolution explicit and robust.
148-176: MolQCA is correctly wired intotech_impl_name.Defining
mol_qca_name = "molQCA"and extendingtech_impl_namesomol_qca_technologymaps to that string makes MolQCA appear consistently in technology descriptions alongside QCA, iNML, and SiDB.
235-240: MolQCA cell-level layout alias and variant integration look correct.
mol_qca_cell_clk_lytand its shared_ptr alias are consistent with the QCA cell-level layout definition, andcell_layout_tnow includesmol_qca_cell_clk_lyt_ptralongside the other technologies. This should allow MolQCA layouts to flow through the existing CLI/IO machinery without structural changes.Also applies to: 327-329
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
23575-23601: Unify QLL writer terminology to “molQCA (mQCA) technology platform”.Both QLL writer docstrings end with “simulator for the mQCA technology platform.” To stay consistent with the rest of the API and clearly introduce the acronym, consider:
- as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical - simulator for the mQCA technology platform. + as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical + simulator for the molQCA (mQCA) technology platform.Apply the same change in
__doc_fiction_write_qll_layout_2.
23286-23305: Fix template-parameter description forwrite_mol_qca_layout_svg(stream overload).The stream overload still documents
Lytas a generic “Cell-level QCA layout type.” while the file overload correctly says “Cell-level molQCA layout type.” Please align the stream overload with the actual semantics:-Template parameter ``Lyt``: - Cell-level QCA layout type. +Template parameter ``Lyt``: + Cell-level molQCA layout type.
🧹 Nitpick comments (1)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
17049-17077: Consider documenting the molQCA predicate helpers.All the
is_*helpers formol_qca_technology(e.g.,is_const_0_cell,is_input_cell,is_crossover_cell_mode, etc.) currently have empty docstrings. Since these are the primary classification predicates exposed into Python, adding one‑line explanations (what each returnstruefor) would noticeably improve the bindings’ usability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(6 hunks)include/fiction/technology/molecular_qca_library.hpp(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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-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:
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:
include/fiction/technology/molecular_qca_library.hpp
🧬 Code graph analysis (1)
include/fiction/technology/molecular_qca_library.hpp (2)
include/fiction/technology/qca_one_library.hpp (1)
determine_port_routing(151-151)include/fiction/technology/fcn_gate_library.hpp (1)
unsupported_gate_orientation_exception(56-60)
🔇 Additional comments (4)
include/fiction/technology/molecular_qca_library.hpp (4)
22-31: Documentation is comprehensive and well-cited.The Doxygen comment properly describes the library's origin, cites the relevant paper, and provides a link to the open-source implementation. This addresses the previous review feedback about using the proper name and citation.
111-168: Port routing logic is sound.The function correctly determines input and output port positions based on directional signals, with sensible fallback behavior for primary inputs/outputs that lack explicit routing. The port positions are consistent with the gate definitions and lookup maps used throughout the file.
176-430: Gate definitions are well-structured.The static gate definitions use clear naming conventions and follow the pattern established in the codebase. While physical correctness requires domain expertise and simulation results (which this library is based on per the documentation), the code structure is sound.
438-572: Lookup maps are comprehensive and well-organized.The port-to-gate mappings cover all necessary rotations and orientations for each gate type. The use of
phmap::flat_hash_mapprovides efficient lookup, and the structure aligns with similar gate libraries in the codebase.
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
include/fiction/technology/molecular_qca_library.hpp (1)
46-54: Fanout size > 2 not handled - clarify error or add support.When
lyt.is_fanout(n)returns true butlyt.fanout_size(n) > 2, the inner condition fails and the function falls through to subsequent checks, eventually throwingunsupported_gate_type_exception. This provides no clear indication that the issue is specifically an unsupported fanout size.Either add explicit handling for larger fanouts (if architecturally supported, as suggested by
qca_one_library.hpphandling fanout_size == 3), or add an explicit check that throws a clear error message for unsupported fanout sizes.For example, to provide a clearer error:
if (lyt.is_fanout(n)) { if (lyt.fanout_size(n) == 2) { return FANOUT_MAP.at(p); } + else + { + throw std::invalid_argument( + fmt::format("Unsupported fanout size {} at tile {}. Only fanout size 2 is supported.", + lyt.fanout_size(n), t)); + } }bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (3)
23565-23569: Unify QLL doc terminology to “molQCA (mQCA) technology platform”.For consistency with the rest of the API, consider expanding the final phrase in this QLL writer docstring from “mQCA technology platform” to “molQCA (mQCA) technology platform”:
-as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical -simulator for the mQCA technology platform. +as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical +simulator for the molQCA (mQCA) technology platform.
23586-23590: Apply the same molQCA terminology fix to the filename overload.The filename-based
write_qll_layoutoverload repeats “mQCA technology platform.” Apply the same wording as above for consistency:-as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical -simulator for the mQCA technology platform. +as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical +simulator for the molQCA (mQCA) technology platform.
23275-23317: AlignLyttemplate description with molQCA-specific writer.The stream overload docstring for
write_mol_qca_layout_svgis molQCA-specific, but the template description still says “Cell-level QCA layout type.” To match the file overload and avoid confusion, update it to molQCA:-Template parameter ``Lyt``: - Cell-level QCA layout type. +Template parameter ``Lyt``: + Cell-level molQCA layout type.
🧹 Nitpick comments (1)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
10980-11009: Clarifygenerate_tile_based_svghelper docstring.The helper docstrings look fine overall, but ending
generate_tile_based_svgwith just “Unused.” is a bit abrupt. If this helper is intentionally internal, consider either dropping that word or expanding it to something like “Currently unused internal helper for potential tile-based molQCA SVG support” to avoid confusing users who inspect the bindings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(6 hunks)include/fiction/technology/molecular_qca_library.hpp(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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-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:
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:
include/fiction/technology/molecular_qca_library.hpp
🔇 Additional comments (3)
include/fiction/technology/molecular_qca_library.hpp (1)
22-25: LGTM: Utility class design is appropriate.The deleted default constructor correctly prevents instantiation of this utility class that exposes only static members and lookup tables.
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
17009-17105: MolQCA technology and gate-library docstrings are consistent.The new descriptions for molQCA cell marks/types and
molecular_qca_library::set_up_gatecorrectly reflect molQCA semantics (tiles, rotation, const cells, I/Os). No changes needed.
19255-19260: Post-layout optimization docstring is clear.The description of assigning via cell mode to wire crossings and the generic
CellLyttemplate parameter look accurate and sufficient.
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
23525-23529: Unify terminology in QLL writer docs: prefer “molQCA (mQCA)”.Both QLL writer overload docstrings still end with “simulator for the mQCA technology platform.” The rest of the API generally uses “molQCA (mQCA)”, so I’d align these for consistency:
-... a physical simulator for the iNML technology platform as well -as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical -simulator for the mQCA technology platform. +... a physical simulator for the iNML technology platform as well +as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical +simulator for the molQCA (mQCA) technology platform.Apply the same change to both overloads’ trailing sentence.
Also applies to: 23546-23550
🧹 Nitpick comments (6)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (3)
10980-11009: Clarify molQCA scope in SVG detail helper docstrings.The
generate_cell_based_svgandgenerate_tile_based_svgdocstrings talk about a generic “clocked cell layout” and don’t mention molQCA, even though they live inwrite_mol_qca_layout_svg_impl. To avoid confusion with legacy QCA SVG writers, consider tightening the wording, e.g.:-static const char* __doc_fiction_detail_write_mol_qca_layout_svg_impl_generate_cell_based_svg = -R"doc(Generates an SVG string representing the cell-based clocked cell -layout and appends it to the output stream.)doc"; +static const char* __doc_fiction_detail_write_mol_qca_layout_svg_impl_generate_cell_based_svg = +R"doc(Generates an SVG fragment representing the cell-based molQCA layout +and appends it to the output stream.)doc"; -static const char* __doc_fiction_detail_write_mol_qca_layout_svg_impl_generate_tile_based_svg = -R"doc(Generates an SVG string representing the tile-based clocked cell -layout and appends it to the output stream. Unused.)doc"; +static const char* __doc_fiction_detail_write_mol_qca_layout_svg_impl_generate_tile_based_svg = +R"doc(Generates an SVG fragment representing the tile-based molQCA layout +and appends it to the output stream. Currently unused.)doc";This keeps internal helpers accurate and clearly molQCA-specific.
17009-17105: Fill in high-level docs formol_qca_technologyandmolecular_qca_library.Many of the new molQCA symbols here (
mol_qca_technology,mol_qca_technology_mol_qca_technology,molecular_qca_library,determine_port_routing) still have completely empty docstrings, while only some enums/values are documented. For Python users,help()will show almost nothing for the core technology and library types.I’d recommend at least short 1–2 line summaries for:
mol_qca_technology(what technology traits it models and that it defines molQCA cell types/marks/modes),molecular_qca_library(that it is the MolQCA gate library and how it is intended to be used).You can base the text on the existing C++ Doxygen comments for these types and then regenerate this header, so the Python bindings surface that context.
23275-23277: Add user-facing docs forwrite_mol_qca_layout_svgoverloads.Both
__doc_fiction_write_mol_qca_layout_svgand_2are empty, so the main Python API for writing molQCA SVGs has no docstring, while the internaldetail_write_mol_qca_layout_svg_impl_*helpers above already describe parts of the behavior.Consider adding short docstrings along the lines of:
- stream overload: writes an SVG representation of a cell-level molQCA layout to a given output stream (cell-based, 10×10 molQCA gates, etc. as appropriate),
- filename overload: same semantics but creates/writes the SVG file.
Reusing or adapting the wording from the corresponding C++ declaration (and from the detail helpers you just documented) will keep everything consistent.
include/fiction/io/write_svg_layout.hpp (3)
448-562: MolQCA SVG constants and templates look good; only cosmetic naming nit.The MolQCA color constants, offsets, and SVG fragments are self-contained and parallel the QCA variants, which is fine. If you ever touch this again, you might align
PI_CELL_MOL_QCA/PO_CELL_MOL_QCAwith theMOL_QCA_*prefix pattern (e.g.,MOL_QCA_PI_CELL) for naming consistency, but it’s strictly optional.
1259-1359: Slightly clearer typing / exhaustiveness for MolQCAcell_colorsindex.
idxis auint64_tthat’s then used to indexcell_colors; functionally this is fine since it’s always in[0, 3], but for readability you could (1) usestd::size_t(ordecltype(cell_colors)::size_type) foridx, and (2) optionally add an assertion if you ever suspect thatis_normal_cell{1..4}might not be mutually exclusive/exhaustive, instead of silently defaulting to index 0.
1661-1671: MolQCA SVG public API has appropriate technology/coordinate guards.The
static_asserts mirror the QCA writer and constrain use to cell‑level MolQCA layouts on unsigned Cartesian coordinates, which should keep misuse and template diagnostics in check. Behavior-wise this looks solid; optionally, you might later add a brief doc comment similar towrite_qca_layout_svgthat mentions the possibleunsupported_cell_type_exceptionon unsupported MolQCA cells.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(6 hunks)include/fiction/io/write_svg_layout.hpp(6 hunks)include/fiction/technology/cell_technologies.hpp(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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-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:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/technology/cell_technologies.hppinclude/fiction/io/write_svg_layout.hpp
🧬 Code graph analysis (2)
include/fiction/technology/cell_technologies.hpp (3)
include/fiction/io/write_svg_layout.hpp (7)
nodiscard(106-109)nodiscard(643-647)nodiscard(658-699)c(888-965)c(888-888)c(1259-1359)c(1259-1259)include/fiction/technology/qca_one_library.hpp (2)
nodiscard(46-111)nodiscard(151-206)include/fiction/layouts/cell_level_layout.hpp (13)
nodiscard(142-148)nodiscard(192-200)nodiscard(208-224)c(162-185)c(162-162)c(192-192)c(241-241)c(252-262)c(252-252)c(269-269)c(285-295)c(285-285)c(302-302)
include/fiction/io/write_svg_layout.hpp (3)
include/fiction/technology/cell_technologies.hpp (16)
c(68-68)c(73-73)c(78-78)c(83-83)c(88-88)c(93-93)c(98-98)c(207-207)c(212-212)c(217-217)c(222-222)c(227-227)c(232-232)c(237-237)c(242-242)c(247-247)include/fiction/io/write_qca_layout.hpp (1)
lyt(198-232)include/fiction/layouts/cartesian_layout.hpp (4)
coord(108-111)coord(108-108)coord(821-829)coord(821-821)
🔇 Additional comments (12)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
19255-19260: Post-layout optimization docstring is clear and specific.The new description for
qca_one_library_post_layout_optimization(“assigns the via cell mode to wire crossings” plus template parameter) is concise and accurately conveys the pass’ effect on the layout. No further changes needed here.include/fiction/technology/cell_technologies.hpp (6)
21-96: LGTM: enum class conversion improves type safety.The conversion from plain enum to
enum classforcell_type, along with the corresponding updates to helper functions andcell_mark, enhances type safety and prevents implicit conversions. The use ofconstreference parameters is appropriate.
212-215: LGTM: Correctly delegates to individual helper functions.The
is_normal_cellimplementation properly delegates to theis_normal_cell1-4helper functions, which is the correct approach as suggested in previous reviews.
217-235: LGTM: Helper functions for NORMAL1-4 are now present.The missing
is_normal_cell1-4helper predicates that were flagged in a previous critical review are now correctly implemented. These functions are required by the SVG writer code.
310-438: LGTM: Consistent enum class conversion for iNML technology.The conversion to
enum classforinml_technologyfollows the same pattern asqca_technologyand maintains consistency across the codebase.
448-539: LGTM: Consistent enum class conversion for SiDB technology.The conversion to
enum classforsidb_technologyfollows the same pattern as other technologies and correctly handles the SiDB-specificLOGICcell type.
284-296: Verify molQCA cell spacing constants against SCERPA/MagCAD documentation.The cell dimensions and spacing constants (CELL_WIDTH=18, CELL_HEIGHT=18, CELL_HSPACE=2, CELL_VSPACE=2) currently match the standard QCA values. Confirm these are appropriate for molecular QCA by consulting the official SCERPA simulator documentation or MagCAD parameters for molQCA technology. If different defaults are required, update these constants accordingly.
include/fiction/io/write_svg_layout.hpp (5)
2-2: Header comment change is fine.Purely informational metadata; no behavioral impact or further action needed.
1364-1410: Cell‑based MolQCA SVG generation mirrors QCA logic correctly.Offsets and
MOL_QCA_CELL_DISTANCEare threaded consistently through both placement and viewBox computation, so behavior should track the existing QCA cell‑based writer with the expected MolQCA geometry. No issues spotted here.
1650-1650: Saferofstreamconstruction fromstd::string_view.Switching to
std::string(filename)instead offilename.data()avoids relying on null‑termination ofstd::string_view; this is the right fix and aligns with modern C++ best practices.
1673-1686: File‑based MolQCA SVG overload correctly delegates to the stream variant.Opening via
std::string(filename), checkingis_open(), and delegating to thestd::ostream&overload is consistent with the QCA and SiDB writers; no concerns here.
1727-1727: Consistent fix for SiDB SVG writer file opening.Using
std::string(filename)here as well brings the SiDB writer in line with the QCA/MolQCA writers and avoids thestd::string_view::data()trap; looks good.
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (3)
23295-23296: Template parameter description should say “molQCA layout type”.The stream overload docstring still describes
Lytas a generic “Cell-level QCA layout type”, while the function is molQCA-specific and the file overload below already uses “Cell-level molQCA layout type.”Consider updating:
-Template parameter ``Lyt``: - Cell-level QCA layout type. +Template parameter ``Lyt``: + Cell-level molQCA layout type.
23576-23580: Unify QLL writer wording: prefer “molQCA (mQCA) technology platform”.Both QLL writer docstrings end with “simulator for the mQCA technology platform.” To match the rest of the API and introduce the acronym, consider:
-as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical -simulator for the mQCA technology platform. +as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical +simulator for the molQCA (mQCA) technology platform.
23597-23601: Apply the same “molQCA (mQCA) technology platform” wording to the file overload.For consistency with the stream overload and the rest of the bindings, mirror the same terminology here:
-as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical -simulator for the mQCA technology platform. +as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical +simulator for the molQCA (mQCA) technology platform.include/fiction/io/write_svg_layout.hpp (1)
1416-1602: Dead code: Remove or markgenerate_tile_based_svgas[[maybe_unused]].This function is never called by
run()(Line 1221 only invokesgenerate_cell_based_svg()), making it unreachable dead code. Additionally, Line 1580 has a latent clock-zone wraparound risk whenczone_up + (latch_delay % lyt.num_clocks()) >= 4, potentially causing out-of-bounds access totile_colorsandtext_colors.If MOL_QCA is not intended to support tile-based layouts:
- Remove this function entirely to eliminate clang-tidy warnings and maintenance burden.
If you plan to enable tile-based layouts in the future:
- Mark the function
[[maybe_unused]]and fix the wraparound issue.Apply this diff to fix the wraparound if keeping the function:
- const auto czone_lo = czone_up + (latch_delay % lyt.num_clocks()); + const auto czone_lo = + (czone_up + (latch_delay % lyt.num_clocks())) % lyt.num_clocks();include/fiction/technology/molecular_qca_library.hpp (1)
56-65: Add support forfanout_size > 2or explicitly reject it.The code only handles
fanout_size(n) == 2(Line 60), butis_fanout(n)returnstruefor any buffer withfanout_size(n) > 1. The QCA library (qca_one_library.hpp) handles bothfanout_size == 2andfanout_size == 3, confirming that larger fanouts are architecturally supported.When
fanout_size(n) > 2, the condition at Line 60 fails, causing the function to fall through and eventually throwunsupported_gate_type_exception(Line 107), providing no clear diagnostic.Add explicit handling or rejection:
if constexpr (fiction::has_is_fanout_v<GateLyt>) { if (lyt.is_fanout(n)) { if (lyt.fanout_size(n) == 2) { return FANOUT_MAP.at(p); } + // TODO: Add support for fanout_size == 3 if needed + throw unsupported_gate_type_exception(t); } }
🧹 Nitpick comments (2)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
17009-17116: MolQCA technology and gate-library docs are solid; helper predicates could be documented later.The
molecular_qca_libraryandset_up_gatedocstrings clearly tie into SIM(7)-MolPDK and MolQCA semantics, including 10×10 tiles and gate realization. If you want richer Python help in the future, you could add one-line descriptions for the variousmol_qca_technology_is_*helpers, but this is optional.include/fiction/technology/cell_technologies.hpp (1)
68-121: Consider pass-by-value for enum class parameters.The predicate functions take
const cell_type&andconst cell_mode¶meters. Since these areenum class : uint8_t(8-bit types), passing by value (cell_type c,cell_mode m) is more idiomatic and may be more efficient than passing by const reference, which requires indirection.This applies to all technology structs in this file (qca_technology, mol_qca_technology, inml_technology, sidb_technology).
Example:
- [[nodiscard]] static constexpr bool is_empty_cell(const cell_type& c) noexcept + [[nodiscard]] static constexpr bool is_empty_cell(cell_type c) noexcept
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(6 hunks)include/fiction/io/write_svg_layout.hpp(6 hunks)include/fiction/technology/cell_technologies.hpp(8 hunks)include/fiction/technology/molecular_qca_library.hpp(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/technology/cell_technologies.hppinclude/fiction/io/write_svg_layout.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:
include/fiction/technology/molecular_qca_library.hpp
🧬 Code graph analysis (2)
include/fiction/technology/cell_technologies.hpp (2)
include/fiction/technology/molecular_qca_library.hpp (2)
nodiscard(47-108)nodiscard(112-168)include/fiction/layouts/cell_level_layout.hpp (13)
nodiscard(142-148)nodiscard(192-200)nodiscard(208-224)c(162-185)c(162-162)c(192-192)c(241-241)c(252-262)c(252-252)c(269-269)c(285-295)c(285-285)c(302-302)
include/fiction/technology/molecular_qca_library.hpp (3)
include/fiction/technology/cell_technologies.hpp (8)
nodiscard(68-71)nodiscard(73-76)nodiscard(78-81)nodiscard(83-86)nodiscard(88-91)nodiscard(93-96)nodiscard(98-101)nodiscard(103-106)include/fiction/technology/qca_one_library.hpp (1)
determine_port_routing(151-151)include/fiction/technology/fcn_gate_library.hpp (1)
unsupported_gate_orientation_exception(56-60)
🔇 Additional comments (12)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (2)
10980-11010: MolQCA SVG implementation helper docstrings look consistent and clear.The helper docstrings for generating cell-based SVGs and description/color pairs align with the public molQCA SVG writers and accurately describe their roles. No changes needed.
19267-19268: QCA ONE post-layout optimization description is concise and accurate.The docstring correctly explains that the optimization assigns the via cell mode at wire crossings, which matches the expected behavior of the QCA ONE library post-layout pass.
include/fiction/io/write_svg_layout.hpp (5)
448-561: LGTM: MOL_QCA constants and SVG templates are well-structured.The MOL_QCA-specific color codes, distances, offsets, and SVG templates (VIA, CONST0/1, CELL, SIMPLE_CELL) follow the same pattern as the existing QCA variants and correctly integrate with the MOL_QCA technology.
1206-1410: LGTM: MOL_QCA cell-based SVG generation is correct.The
write_mol_qca_layout_svg_implclass correctly implements cell-based MOL_QCA layout rendering:
generate_description_colorproperly dispatches on cell types (including NORMAL1-4 viais_normal_cell1-4helpers).- Crossings are explicitly unsupported and throw an exception (Line 1322).
generate_cell_based_svguses appropriate MOL_QCA offsets and distances.
1661-1711: LGTM: Public API for MOL_QCA SVG rendering is correct.The
write_mol_qca_layout_svgfunctions properly:
- Restrict to cell-based layouts (consistent with
run()behavior).- Use
has_mol_qca_technology_v<Lyt>static_assert (Line 1677).- Correctly convert
filenametostd::string(Line 1702).- Document the 10×10 uniform gate size constraint.
The docstrings have been adjusted from the QCA variants to reflect MOL_QCA-specific constraints.
1650-1650: LGTM: Safe conversion fromstring_viewtostd::string.Changing
filename.data()tostd::string(filename)ensures a null-terminated string forstd::ofstream, avoiding potential issues withstring_view::data()not being null-terminated.
1752-1752: LGTM: Safe conversion fromstring_viewtostd::string.Changing
filename.data()tostd::string(filename)ensures a null-terminated string forstd::ofstream, avoiding potential issues withstring_view::data()not being null-terminated.include/fiction/technology/cell_technologies.hpp (2)
143-300: Verify MOL_QCA cell dimensions against MagCAD/SCEERPA defaults.The
mol_qca_technologystruct correctly defines NORMAL1-4 cell types for different clock zones and provides all required helper predicates. However, the cell dimensions (CELL_WIDTH=18, CELL_HEIGHT=18, CELL_HSPACE=2, CELL_VSPACE=2 at Lines 284-296) mirror the QCA defaults from QCADesigner rather than MOL_QCA-specific values.According to the PR description and
molecular_qca_library.hpp, this library is based on the SIM(7)-MolPDK standard-cell library with 10×10 cell blocks. Please confirm whether these dimensions should instead use defaults from MagCAD/SCEERPA or the physically simulated MolPDK parameters.Based on learnings, a past review suggested using MagCAD/SCEERPA defaults for MOL_QCA cell dimensions.
212-215: LGTM:is_normal_cellcorrectly aggregates all normal cell variants.The function now returns
truefor all NORMAL1-4 variants, addressing the concern from past reviews that it should recognize any normal cell type, not just a single NORMAL variant.include/fiction/technology/molecular_qca_library.hpp (3)
22-35: LGTM: Well-documented MolQCA gate library with proper citation.The docstring correctly references the SIM(7)-MolPDK library, the associated LANANO 2025 paper, the SCERPA simulation tool, and the open-source repository. The 10×10 tile size is consistent throughout the implementation.
112-168: LGTM: Port routing logic is sound for 10×10 tiles.The
determine_port_routingfunction correctly:
- Maps incoming/outgoing signals to appropriate port positions on the 10×10 tile edges (N, E, S, W).
- Provides sensible fallbacks for tiles without connectors (e.g., primary inputs/outputs).
- Uses noexcept to signal it cannot fail.
176-572: LGTM: Gate templates and lookup maps are comprehensive.The gate templates (STRAIGHT_INVERTER, CONJUNCTION, MAJORITY, FAN_OUT_1_2, etc.) and corresponding lookup maps (WIRE_MAP, INVERTER_MAP, etc.) correctly:
- Define 10×10 cell-level gate layouts using character symbols ('a'-'d' for NORMAL1-4 clock zones, '0'/'1' for constants, 'i'/'o' for I/O).
- Provide all necessary rotations (0°, 90°, 180°, 270°) and orientations.
- Map port lists to appropriate gate representations.
The implementation aligns with the SIM(7)-MolPDK library design.
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)
23291-23292: Template parameter description inconsistency.The stream overload's template parameter says "Cell-level QCA layout type" but should say "Cell-level molQCA layout type" to match the file overload at line 23314.
Template parameter ``Lyt``: - Cell-level QCA layout type. + Cell-level molQCA layout type.
23571-23579: Terminology inconsistency: "mQCA" vs "molQCA (mQCA)".Both QLL writer docstrings use "mQCA technology platform" but should use "molQCA (mQCA)" for consistency with the rest of the API terminology.
R"doc(Writes a cell-level QCA, molQCA or iNML layout to a qll file that is used by ToPoliNano & MagCAD (https://topolinano.polito.it/), an EDA tool and a physical simulator for the iNML technology platform as well as SCERPA (https://ieeexplore.ieee.org/document/8935211), a physical -simulator for the mQCA technology platform. +simulator for the molQCA (mQCA) technology platform.Also applies to lines 23592-23597.
🧹 Nitpick comments (3)
include/fiction/technology/molecular_qca_library.hpp (1)
56-69: Fanout handling for size 2 and 3 returns the same gate lookup.The conditions for
fanout_size(n) == 2andfanout_size(n) == 3both returnFANOUT_MAP.at(p)identically. While the FANOUT_MAP does contain distinct entries for 2-output and 3-output fanouts (lines 560-590), this duplicated conditional structure is confusing. Consider consolidating:if constexpr (fiction::has_is_fanout_v<GateLyt>) { if (lyt.is_fanout(n)) { - if (lyt.fanout_size(n) == 2) - { - return FANOUT_MAP.at(p); - } - if (lyt.fanout_size(n) == 3) + if (const auto fs = lyt.fanout_size(n); fs == 2 || fs == 3) { return FANOUT_MAP.at(p); } } }This also makes it explicit that fanout sizes other than 2 or 3 will fall through to
unsupported_gate_type_exception.include/fiction/io/write_svg_layout.hpp (2)
1231-1251: Remove unused type aliases.These type aliases (
tile_description,coord_to_tile_mapping,coord_to_cell_list_mapping,svg_latch,coord_to_latch_mapping) are copied fromwrite_qca_layout_svg_implbut are never used in this class sincerun()only callsgenerate_cell_based_svg(). This was noted in a past review comment that tile-based generation is out of scope for MOL-QCA.- /** - * Alias for an SVG description of a tile containing also its clock zone. - */ - using tile_description = std::pair<std::string, typename Lyt::clock_number_t>; - /** - * Maps coordinates of tiles to their string representation and their clock zone. - */ - using coord_to_tile_mapping = std::unordered_map<coordinate<Lyt>, tile_description>; - /** - * Maps coordinates of tiles to a string representation of the cells contained within them. - */ - using coord_to_cell_list_mapping = std::unordered_map<coordinate<Lyt>, std::string>; - /** - * Alias for an SVG description of a latch tile containing also its clock zone and its offset. - */ - using svg_latch = std::tuple<std::string, typename Lyt::clock_number_t, uint32_t>; - /** - * Maps coordinates of latch tiles to tuples containing their string representation, their clock zone, - * and their latch offset. - */ - using coord_to_latch_mapping = std::unordered_map<coordinate<Lyt>, svg_latch>;
1286-1307: Consider adding a defensive assertion for the clock zone index fallback.The fallback
return uint64_t{0}at line 1304 could silently mask an unexpected cell type that passesis_normal_cellbut doesn't match any of theis_normal_cell1/2/3/4predicates. While this may be impossible given the technology definition, an assertion would catch potential future inconsistencies:const uint64_t idx = [&ct] { if (Lyt::technology::is_normal_cell1(ct)) { return uint64_t{0}; } if (Lyt::technology::is_normal_cell2(ct)) { return uint64_t{1}; } if (Lyt::technology::is_normal_cell3(ct)) { return uint64_t{2}; } if (Lyt::technology::is_normal_cell4(ct)) { return uint64_t{3}; } + assert(false && "Normal cell does not match any clock zone"); return uint64_t{0}; }();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(6 hunks)include/fiction/io/write_svg_layout.hpp(6 hunks)include/fiction/technology/molecular_qca_library.hpp(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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-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:
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hppinclude/fiction/io/write_svg_layout.hpp
🧬 Code graph analysis (1)
include/fiction/io/write_svg_layout.hpp (2)
include/fiction/technology/cell_technologies.hpp (16)
c(68-68)c(73-73)c(78-78)c(83-83)c(88-88)c(93-93)c(98-98)c(207-207)c(212-212)c(217-217)c(222-222)c(227-227)c(232-232)c(237-237)c(242-242)c(247-247)include/fiction/layouts/coordinates.hpp (8)
format(1083-1086)format(1083-1083)format(1099-1102)format(1099-1099)format(1115-1118)format(1115-1115)os(295-299)os(295-295)
🔇 Additional comments (10)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (3)
10980-11005: LGTM!The implementation detail docstrings are appropriately documented. The
generate_description_colormethod has clear parameter and return documentation.
17077-17112: Well-documented MolQCA library with proper citations.The
molecular_qca_librarydocstring provides comprehensive documentation including the academic reference to SIM(7)-MolPDK, a link to the open-source implementation, and clear parameter/return documentation forset_up_gate. The terminology correctly uses "molQCA gate" throughout.
19262-19267: LGTM!The clarification about via cell mode assignment for wire crossings improves the documentation.
include/fiction/technology/molecular_qca_library.hpp (3)
115-172: LGTM!The
determine_port_routingfunction correctly determines input/output ports for the 10×10 tile grid with appropriate fallback handling for primary I/O tiles.
581-590: Verify: FAN_OUT_1_3 rotations appear to be missing.All four orientations of
FAN_OUT_1_3map to the same unrotated gate. Looking at the gate pattern (lines 362-374), it has input from the north and outputs to east, south, and west—this is not rotationally symmetric. Compare with the fanout-2 entries which userotate_90,rotate_180, androtate_270variants.If the gate is meant to be symmetric, this is fine. Otherwise, rotated variants should be applied:
// fanout 3 // identity orientation {{{port_position(4, 0)}, {port_position(9, 4), port_position(5, 9), port_position(0, 5)}}, FAN_OUT_1_3}, // rotated 90° - {{{port_position(9, 4)}, {port_position(5, 9), port_position(0, 5), port_position(4, 0)}}, FAN_OUT_1_3}, + {{{port_position(9, 4)}, {port_position(5, 9), port_position(0, 5), port_position(4, 0)}}, rotate_90(FAN_OUT_1_3)}, // rotated 180° - {{{port_position(5, 9)}, {port_position(0, 5), port_position(4, 0), port_position(9, 4)}}, FAN_OUT_1_3}, + {{{port_position(5, 9)}, {port_position(0, 5), port_position(4, 0), port_position(9, 4)}}, rotate_180(FAN_OUT_1_3)}, // rotated 270° - {{{port_position(0, 5)}, {port_position(4, 0), port_position(9, 4), port_position(5, 9)}}, FAN_OUT_1_3}, + {{{port_position(0, 5)}, {port_position(4, 0), port_position(9, 4), port_position(5, 9)}}, rotate_270(FAN_OUT_1_3)},
22-31: LGTM!Good documentation with proper academic citation and link to the open-source implementation.
include/fiction/io/write_svg_layout.hpp (4)
448-561: LGTM!The MOL_QCA SVG constants and templates are well-structured with appropriate scaling (11.5 cell distance for 10×10 tiles vs 23 for standard QCA).
1364-1410: LGTM!The
generate_cell_based_svgimplementation correctly uses MOL_QCA-specific constants for cell distance and offsets, following the established pattern from the QCA implementation.
1481-1491: LGTM!The static assertions correctly enforce MOL_QCA technology constraint, addressing the past review comment about the missing technology guard.
1458-1458: LGTM!Good fix converting
std::string_viewtostd::stringfor thestd::ofstreamconstructor, ensuring proper null-termination.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
include/fiction/io/write_svg_layout.hpp (2)
1206-1389: MolQCA SVG writer implementation looks consistent with QCA, but shares duplicated structure.The control flow and cell classification in
write_mol_qca_layout_svg_impllook consistent with the existing QCA writer (correct handling of synchronization elements, explicit rejection of crossings, and appropriate use of MolQCA‑specific templates and distances). However, a substantial portion ofgenerate_description_colorandgenerate_cell_based_svgmirrors the QCA implementation almost verbatim.To keep maintenance effort down as both writers evolve, consider extracting shared pieces (e.g., common formatting and traversal logic) into a helper or base facility that both QCA and MolQCA writers can reuse.
1447-1469: Fix MolQCA ostream overload docstring to refer to MolQCA, not generic QCA.The implementation and
static_assert(has_mol_qca_technology_v<Lyt>, ...)clearly target MolQCA layouts, but the Doxygen line* @tparam Lyt Cell-level QCA layout type.still refers to “QCA” generically. Please update this to “Cell-level molQCA layout type” (or similar) to avoid confusion between the QCA and MolQCA writers.
A minimal change:
- * @tparam Lyt Cell-level QCA layout type. + * @tparam Lyt Cell-level molQCA layout type.
🧹 Nitpick comments (1)
include/fiction/io/write_svg_layout.hpp (1)
448-562: Document MolQCA SVG constants to align with guidelines.The new
MOL_QCA_*constants (clock-zone colors, distances, offsets, and cell templates) are currently undocumented. A brief Doxygen block describing that this group defines the MolQCA color scheme and geometry would help future readers and satisfies the “Doxygen for all new symbols” guideline. As per coding guidelines, these new symbols should have a/** ... */description.You could, for example, add a brief group-level comment:
-inline constexpr const char* SIMPLE_CELL = "<g\n" - "transform=\"translate({1},{2})\">\n" - "<rect\n" - "ry=\"1.5\"\n" - "y=\"112.92032\"\n" - "x=\"185.86807\"\n" - "height=\"20\"\n" - "width=\"20\"\n" - "style=\"fill:#{0};stroke:#000000;\" />\n" - "</g>"; - -// molQCA +inline constexpr const char* SIMPLE_CELL = "<g\n" + "transform=\"translate({1},{2})\">\n" + "<rect\n" + "ry=\"1.5\"\n" + "y=\"112.92032\"\n" + "x=\"185.86807\"\n" + "height=\"20\"\n" + "width=\"20\"\n" + "style=\"fill:#{0};stroke:#000000;\" />\n" + "</g>"; + +/** @brief MolQCA-specific SVG color codes, geometry, and cell templates. + * + * Defines the MolQCA clock‑zone colors, intra‑cell distances, offsets, and + * basic glyph templates (vias, constants, and cells) used by + * write_mol_qca_layout_svg_impl. + */ +// molQCA
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
include/fiction/io/write_svg_layout.hpp(6 hunks)include/fiction/technology/molecular_qca_library.hpp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- include/fiction/technology/molecular_qca_library.hpp
🧰 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/io/write_svg_layout.hpp
**/*.hpp
📄 CodeRabbit inference engine (AGENTS.md)
Use
.hppextension for C++ header files and include guards with#ifndef FICTION_FILENAME_HPPformat
Files:
include/fiction/io/write_svg_layout.hpp
🧠 Learnings (1)
📚 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/io/write_svg_layout.hpp
⏰ 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). (12)
- GitHub Check: 🍎 macos-15 with clang++
- GitHub Check: 🛞 Wheels for windows-latest
- GitHub Check: 🛞 Wheels for ubuntu-24.04-arm
- GitHub Check: Analyze cpp
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-16
- GitHub Check: 🐧 ubuntu-24.04 with clang++-15
- GitHub Check: 🐧 ubuntu-22.04 with g++-10
- GitHub Check: 🐧 ubuntu-22.04 with clang++-14
- GitHub Check: 🐧 ubuntu-22.04 with clang++-15
- GitHub Check: 🐧 ubuntu-22.04 with g++-11
- GitHub Check: 🐧 ubuntu-24.04 with g++-13
- GitHub Check: 🐳 Build and publish Docker image
🔇 Additional comments (3)
include/fiction/io/write_svg_layout.hpp (3)
1436-1436: Safer construction ofstd::ofstreamfromstd::string_view.Switching to
std::ofstream os{std::string(filename), std::ofstream::out};avoids relying onstd::string_view::data()and ensures the stream owns a stablestd::stringpath. This is a good, portable fix.
1471-1497: File-based MolQCA writer overload matches QCA pattern and looks good.This overload mirrors the QCA/SiDB file-based writers: it opens a stream from
std::string(filename), checksis_open(), forwards to the ostream overload (where the technology guards live), and closes explicitly. The behavior and error semantics are consistent with the rest of the API.
1538-1538: Consistentstd::string_viewhandling in SiDB writer.Using
std::ofstream os{std::string(filename), std::ofstream::out};here aligns the SiDB writer with the QCA/MolQCA functions and avoids any ambiguity aroundstd::string_view::data()usage.
4 participants
Description
Add molecular QCA as a technology for cartesian layouts. In this PR the library is added with the corresponding changes for printing
.svgs, generate.qll-files and adding the command to the cli.Fixes # (issue)
Checklist:
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.