Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐍 layouts using cube coordinates #642

Draft
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

simon1hofmann
Copy link
Collaborator

Description

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have added a changelog entry.
  • I have created/adjusted the Python bindings for any new or updated functionality.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@simon1hofmann simon1hofmann marked this pull request as draft January 22, 2025 16:04
@simon1hofmann
Copy link
Collaborator Author

simon1hofmann commented Jan 22, 2025

As all of the exposed layouts (cartesian_layout, clocked_layout, gate_level_layout, etc.) are currently all based on the offset_coordinate type, this would be a way to extend it to other coordinate types while still being backwards compatible and using offset_coordinate as the default.

What are your opinions on this approach? @Drewniok @marcelwa

We could also go for e.g. cartesian_layout_offset_coordinate for more consistency, but would then loose backwards compatibility.
Passing the coordinate type as a string would also be possible, but we would have to decide on an ordering then:
cartesian_gate_layout((2, 8, 0), "2DDWave", "Layout", "cube")?

Copy link

codecov bot commented Jan 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.12%. Comparing base (c47c5b0) to head (24fa168).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #642   +/-   ##
=======================================
  Coverage   98.11%   98.12%           
=======================================
  Files         235      235           
  Lines       36046    36046           
  Branches     1754     1755    +1     
=======================================
+ Hits        35368    35370    +2     
+ Misses        678      675    -3     
- Partials        0        1    +1     

see 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c47c5b0...24fa168. Read the comment docs.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@marcelwa
Copy link
Collaborator

As all of the exposed layouts (cartesian_layout, clocked_layout, gate_level_layout, etc.) are currently all based on the offset_coordinate type, this would be a way to extend it to other coordinate types while still being backwards compatible and using offset_coordinate as the default.

Thank you for bringing up this issue and offering a solution plus implementation. This is very much appreciated.

What are your opinions on this approach? @Drewniok @marcelwa

I like added flexibility in general if it helps solve a real problem. Are you (or is someone) currently restricted by the defaulted offset coordinate type? What is the use case that could not be achieved before?

We could also go for e.g. cartesian_layout_offset_coordinate for more consistency, but would then loose backwards compatibility. Passing the coordinate type as a string would also be possible, but we would have to decide on an ordering then: cartesian_gate_layout((2, 8, 0), "2DDWave", "Layout", "cube")?

As far as I know, the more "Pythonic" way would be to pass the coordinate type as an argument (with "offset" being the default). I would prefer that as I see it as a "cleaner" solution. And I agree with your ordering of arguments. Only comment would be to use named parameters, e.g.,

cartesian_gate_layout(aspect_ratio=(2, 8, 0), scheme="2DDWave", name="Layout", coordinate_type="cube")

which increases readability and gives even more meaning to the code.

I look forward to reading @Drewniok's opinions.

@Drewniok
Copy link
Collaborator

As all of the exposed layouts (cartesian_layout, clocked_layout, gate_level_layout, etc.) are currently all based on the offset_coordinate type, this would be a way to extend it to other coordinate types while still being backwards compatible and using offset_coordinate as the default.

Thank you for bringing up this issue and offering a solution plus implementation. This is very much appreciated.

What are your opinions on this approach? @Drewniok @marcelwa

I like added flexibility in general if it helps solve a real problem. Are you (or is someone) currently restricted by the defaulted offset coordinate type? What is the use case that could not be achieved before?

We could also go for e.g. cartesian_layout_offset_coordinate for more consistency, but would then loose backwards compatibility. Passing the coordinate type as a string would also be possible, but we would have to decide on an ordering then: cartesian_gate_layout((2, 8, 0), "2DDWave", "Layout", "cube")?

As far as I know, the more "Pythonic" way would be to pass the coordinate type as an argument (with "offset" being the default). I would prefer that as I see it as a "cleaner" solution. And I agree with your ordering of arguments. Only comment would be to use named parameters, e.g.,

cartesian_gate_layout(aspect_ratio=(2, 8, 0), scheme="2DDWave", name="Layout", coordinate_type="cube")

which increases readability and gives even more meaning to the code.

I look forward to reading @Drewniok's opinions.

I talked to Simon about this, and the problem is that we cannot currently implement the Python bindings for QuickTrace etc. because they require cube coordinates. Since in the current version of #634, apply_parameterized_gate_library is also based on cube coordinates, we even need it for that (sure, it might be an option to restructure the function again, but I don't see how at the moment). Since I failed in #546 to successfully add the cube coordinate to all functions, especially our SiDB functions, I asked him if he could try again.

Long story short, I think it would be very useful to have cube coordinates in the Python bindings!

I think the "Pythonic" way would be great indeed

Thanks @simon1hofmann!

@simon1hofmann
Copy link
Collaborator Author

@marcelwa @Drewniok

I now created new layout classes all the way from coordinate layouts to cell-level layouts, which can be used like this:

  • cartesian_layout((9, 9, 1)) or cartesian_layout((9, 9, 1), "offset") or cartesian_layout((9, 9, 1), coordinate_type="offset")
  • cartesian_layout((9, 9, 1), "cube") or cartesian_layout((9, 9, 1), coordinate_type="cube")
  • clocked_cartesian_layout((2, 2, 0), "2DDWave", "cube")
  • cartesian_gate_layout((2, 2, 0), "2DDWave", "Layout", "cube") or cartesian_gate_layout(dimension=(2, 2, 0), scheme_name="2DDWave", layout_name="Layout", coordinate_type="cube")
  • sidb_layout((9, 9, 1), "OPEN", "Layout", "cube")

For the cell-level layouts, I only exposed layouts using SiDB technology so far.

There are still some open problems:

  • How to cast cube coordinates to signals
  • Making sure the C++ implementation also handles cube coordinates correctly (for example the .north() function only worked for offset coordinates)

One discussion I had with @Drewniok:
Currently, all cell-level layouts are based on the Cartesian grid, is that desired? I'm a little bit confused as on a gate-level abstraction, we are using hexagonal layouts.

Looking forward to your feedback and comments.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

*/
explicit constexpr operator uint64_t() const noexcept
{
return (((((((static_cast<uint64_t>(d)) << 1ull) | z) << 31ull) | y) << 31ull) | x);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]

        return (((((((static_cast<uint64_t>(d)) << 1ull) | z) << 31ull) | y) << 31ull) | x);
                                                                                         ^

*/
explicit constexpr operator uint64_t() const noexcept
{
return (((((((static_cast<uint64_t>(d)) << 1ull) | z) << 31ull) | y) << 31ull) | x);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]

        return (((((((static_cast<uint64_t>(d)) << 1ull) | z) << 31ull) | y) << 31ull) | x);
                                                                          ^

*/
explicit constexpr operator uint64_t() const noexcept
{
return (((((((static_cast<uint64_t>(d)) << 1ull) | z) << 31ull) | y) << 31ull) | x);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]

        return (((((((static_cast<uint64_t>(d)) << 1ull) | z) << 31ull) | y) << 31ull) | x);
                                                           ^

@simon1hofmann simon1hofmann changed the title 🐍 layouts using cube and siqad coordinates 🐍 layouts using cube coordinates Jan 25, 2025
@marcelwa
Copy link
Collaborator

@marcelwa @Drewniok

I now created new layout classes all the way from coordinate layouts to cell-level layouts, which can be used like this:

  • cartesian_layout((9, 9, 1)) or cartesian_layout((9, 9, 1), "offset") or cartesian_layout((9, 9, 1), coordinate_type="offset")
  • cartesian_layout((9, 9, 1), "cube") or cartesian_layout((9, 9, 1), coordinate_type="cube")
  • clocked_cartesian_layout((2, 2, 0), "2DDWave", "cube")
  • cartesian_gate_layout((2, 2, 0), "2DDWave", "Layout", "cube") or cartesian_gate_layout(dimension=(2, 2, 0), scheme_name="2DDWave", layout_name="Layout", coordinate_type="cube")
  • sidb_layout((9, 9, 1), "OPEN", "Layout", "cube")

Usability-wise this looks amazing! Many thanks for your hard work on the implementation! 🙏

For the cell-level layouts, I only exposed layouts using SiDB technology so far.

Will you expose the remaining technologies as well?

There are still some open problems:

  • How to cast cube coordinates to signals

For this to work, the type of signal has to change following the passed coordinate type. It is not sufficient to have a signal be only 64 bits if we want it to store a cube coordinate.

  • Making sure the C++ implementation also handles cube coordinates correctly (for example the .north() function only worked for offset coordinates)

I see that this also requires some extra work. Do you need help with this?

One discussion I had with @Drewniok: Currently, all cell-level layouts are based on the Cartesian grid, is that desired? I'm a little bit confused as on a gate-level abstraction, we are using hexagonal layouts.

This is desired. All cell-level grids are represented via Cartesian coordinates. @Drewniok and I were discussing switching the underlying coordinates for SiDB layouts to lattice vectors. They are technically more appropriate to define the crystal structures. Plus, they encode distance information as well. For more information, see: https://www.cda.cit.tum.de/files/eda/2024_ieee_nano_alternative_silicon_orientations.pdf

@simon1hofmann
Copy link
Collaborator Author

simon1hofmann commented Jan 28, 2025

Will you expose the remaining technologies as well?

Will also do that, the implementation should be straightforward.

For this to work, the type of signal has to change following the passed coordinate type. It is not sufficient to have a signal be only 64 bits if we want it to store a cube coordinate.

Do you have any recommendations on how we should do this? I tried some approaches, but did not really figure out a good way to do it other than treating it as an offset coordinate.

I see that this also requires some extra work. Do you need help with this?

I will try to fix all of the issues by myself, but feedback would be much appreciated once I'm done 🙏

This is desired. All cell-level grids are represented via Cartesian coordinates. @Drewniok and I were discussing switching the underlying coordinates for SiDB layouts to lattice vectors. They are technically more appropriate to define the crystal structures. Plus, they encode distance information as well. For more information, see: https://www.cda.cit.tum.de/files/eda/2024_ieee_nano_alternative_silicon_orientations.pdf

Got it 👍

@marcelwa
Copy link
Collaborator

For this to work, the type of signal has to change following the passed coordinate type. It is not sufficient to have a signal be only 64 bits if we want it to store a cube coordinate.

Do you have any recommendations on how we should do this? I tried some approaches, but did not really figure out a good way to do it other than treating it as an offset coordinate.

I have some ideas in my head but I can't tell you for sure if they'll work properly. I would have to play around with it myself a bit, most likely.

@simon1hofmann simon1hofmann self-assigned this Jan 30, 2025
@simon1hofmann
Copy link
Collaborator Author

simon1hofmann commented Jan 30, 2025

@marcelwa Would really appreciate feedback to the implementation of aspect_ratio (can be found in coordinates.hpp) before I adapt the bindings, write tests etc.

Copy link
Collaborator

@marcelwa marcelwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting this work forward and investing your time here. I exclusively checked out the aspect_ratio type as you requested. Please find my comments below 🙂

Comment on lines 1057 to 1064
/**
* Struct representing the aspect ratio of the cartesian layout.
*
* The `aspect_ratio` struct defines the starting and ending coordinates, effectively
* determining the size and position of the layout within a coordinate space.
*/
template <typename CoordinateType>
struct aspect_ratio
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design decision: do we want to make this a real class? How do we plan to interact with it? Fetch start and end directly or via getters?

Comment on lines 1066 to 1067
CoordinateType start;
CoordinateType end;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docstrings

Comment on lines 1156 to 1167
/**
* Equality operator for aspect_ratio.
*
* Compares two `aspect_ratio` instances for equality based on their end coordinates.
*
* @param other The other aspect_ratio instance to compare against.
* @return `true` if both aspect_ratios have the same end coordinates; `false` otherwise.
*/
bool operator==(const aspect_ratio& other) const noexcept
{
return (x() == other.x()) && (y() == other.y()) && (z() == other.z());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you define equality, you should also define inequality. However, if you take min and max into consideration for both, both can be generated automatically via [[nodiscard]] bool operator==(const aspect_ratio& other) const noexcept = default;

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

}

template <typename CoordLyt>
fiction::aspect_ratio<CoordLyt> extract_aspect_ratio(pybind11::tuple dimension)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::aspect_ratio" is directly included [misc-include-cleaner]

bindings/mnt/pyfiction/include/pyfiction/layouts/cartesian_layout.hpp:7:

- #include "pyfiction/documentation.hpp"
+ #include "fiction/layouts/coordinates.hpp"
+ #include "pyfiction/documentation.hpp"

@@ -6,6 +6,7 @@
#define FICTION_CARTESIAN_LAYOUT_HPP

#include "fiction/layouts/coordinates.hpp"
#include "fiction/traits.hpp"
#include "fiction/utils/range.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: included header traits.hpp is not used directly [misc-include-cleaner]

Suggested change
#include "fiction/utils/range.hpp"
#include "fiction/utils/range.hpp"

Drewniok and others added 2 commits February 3, 2025 18:16
…ordinates

# Conflicts:
#	bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp
#	include/fiction/algorithms/physical_design/apply_gate_library.hpp
Signed-off-by: GitHub Actions <[email protected]>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -43,6 +43,8 @@ class apply_gate_library_impl
gate_lyt{lyt},
cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>' [clang-diagnostic-error]

            cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
            ^
Additional context

include/fiction/algorithms/physical_design/apply_gate_library.hpp:268: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<>>>>>::apply_gate_library_impl' requested here

    detail::apply_gate_library_impl<CellLyt, GateLibrary, GateLyt> p{lyt};
                                                                   ^

test/io/write_sqd_layout.cpp:175: in instantiation of function template specialization 'fiction::apply_gate_library<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<>>>>>' requested here

    const auto c_layout = apply_gate_library<sidb_layout, sidb_bestagon_library>(g_layout);
                          ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'const cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<>>>' for 1st argument

class cell_level_layout : public ClockedLayout
      ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit move constructor) not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<>>>' for 1st argument

class cell_level_layout : public ClockedLayout
      ^

include/fiction/layouts/cell_level_layout.hpp:95: candidate constructor not viable: no known conversion from 'aspect_ratio<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>' to 'const aspect_ratio' for 1st argument

    explicit cell_level_layout(const typename ClockedLayout::aspect_ratio& ar = {}, const std::string& name = "",
             ^

include/fiction/layouts/cell_level_layout.hpp:124: candidate constructor not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'std::shared_ptr<cell_level_layout_storage>' (aka 'shared_ptr<cell_level_layout_storagefiction::offset::ucoord_t>') for 1st argument

    explicit cell_level_layout(std::shared_ptr<cell_level_layout_storage<cell>> s) : strg{std::move(s)} {}
             ^

include/fiction/layouts/cell_level_layout.hpp:130: candidate constructor not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'const fiction::clocked_layout<fiction::cartesian_layout<>>' for 1st argument

    explicit cell_level_layout(const ClockedLayout& lyt) :
             ^

include/fiction/layouts/cell_level_layout.hpp:112: candidate constructor not viable: requires at least 2 arguments, but 1 was provided

    cell_level_layout(const typename ClockedLayout::aspect_ratio& ar, const clocking_scheme<cell>& scheme,
    ^

@@ -43,6 +43,8 @@ class apply_gate_library_impl
gate_lyt{lyt},
cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>' [clang-diagnostic-error]

            cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
            ^
Additional context

include/fiction/algorithms/physical_design/apply_gate_library.hpp:268: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>::apply_gate_library_impl' requested here

    detail::apply_gate_library_impl<CellLyt, GateLibrary, GateLyt> p{lyt};
                                                                   ^

test/io/print_layout.cpp:750: in instantiation of function template specialization 'fiction::apply_gate_library<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>' requested here

ll_layout_or       = apply_gate_library<sidb_cell_clk_lyt, sidb_bestagon_library>(layout);
                     ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'const cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<>>>' for 1st argument

class cell_level_layout : public ClockedLayout
      ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit move constructor) not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<>>>' for 1st argument

class cell_level_layout : public ClockedLayout
      ^

include/fiction/layouts/cell_level_layout.hpp:95: candidate constructor not viable: no known conversion from 'aspect_ratio<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>' to 'const aspect_ratio' for 1st argument

    explicit cell_level_layout(const typename ClockedLayout::aspect_ratio& ar = {}, const std::string& name = "",
             ^

include/fiction/layouts/cell_level_layout.hpp:124: candidate constructor not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'std::shared_ptr<cell_level_layout_storage>' (aka 'shared_ptr<cell_level_layout_storagefiction::offset::ucoord_t>') for 1st argument

    explicit cell_level_layout(std::shared_ptr<cell_level_layout_storage<cell>> s) : strg{std::move(s)} {}
             ^

include/fiction/layouts/cell_level_layout.hpp:130: candidate constructor not viable: no known conversion from 'aspect_ratio<cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>>' to 'const fiction::clocked_layout<fiction::cartesian_layout<>>' for 1st argument

    explicit cell_level_layout(const ClockedLayout& lyt) :
             ^

include/fiction/layouts/cell_level_layout.hpp:112: candidate constructor not viable: requires at least 2 arguments, but 1 was provided

    cell_level_layout(const typename ClockedLayout::aspect_ratio& ar, const clocking_scheme<cell>& scheme,
    ^

@@ -43,6 +43,8 @@ class apply_gate_library_impl
gate_lyt{lyt},
cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>' [clang-diagnostic-error]

            cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
            ^
Additional context

include/fiction/algorithms/physical_design/apply_gate_library.hpp:268: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>::apply_gate_library_impl' requested here

    detail::apply_gate_library_impl<CellLyt, GateLibrary, GateLyt> p{lyt};
                                                                   ^

test/io/print_layout.cpp:277: in instantiation of function template specialization 'fiction::apply_gate_library<fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>' requested here

layout.create_or({}, {}, {0, 0});
                                                                                                    ^

include/fiction/technology/sidb_lattice.hpp:37: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'aspect_ratio<sidb_lattice<sidb_100_lattice, cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>, false>>' to 'const sidb_lattice<sidb_100_lattice, cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<>>>>' for 1st argument

class sidb_lattice<LatticeOrientation, Lyt, false> : public Lyt
      ^

include/fiction/technology/sidb_lattice.hpp:37: candidate constructor (the implicit move constructor) not viable: no known conversion from 'aspect_ratio<sidb_lattice<sidb_100_lattice, cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>, false>>' to 'sidb_lattice<sidb_100_lattice, cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<>>>>' for 1st argument

class sidb_lattice<LatticeOrientation, Lyt, false> : public Lyt
      ^

include/fiction/technology/sidb_lattice.hpp:55: candidate constructor not viable: no known conversion from 'aspect_ratio<sidb_lattice<sidb_100_lattice, cell_level_layout<sidb_technology, clocked_layout<cartesian_layout<ucoord_t>>>, false>>' to 'const fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>' for 1st argument

    explicit sidb_lattice(const Lyt& layout) : Lyt(layout)
             ^

include/fiction/technology/sidb_lattice.hpp:66: candidate constructor not viable: no known conversion from 'aspect_ratio<fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>>' to 'const aspect_ratio' for 1st argument

    explicit sidb_lattice(const aspect_ratio_t<Lyt>& ar, const std::string& name = "") : Lyt(ar, name)
             ^

include/fiction/technology/sidb_lattice.hpp:45: candidate constructor not viable: requires 0 arguments, but 1 was provided

    explicit sidb_lattice() : Lyt()
             ^

* @param y The y-coordinate value.
*/
template <typename X, typename Y>
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>' [clang-diagnostic-error]

    aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
                                           ^
Additional context

include/fiction/layouts/cell_level_layout.hpp:95: candidate constructor not viable: no known conversion from 'decltype(this->max.x())' (aka 'unsigned long') to 'const typename clocked_layout<cartesian_layout<>>::aspect_ratio' (aka 'const aspect_ratiofiction::offset::ucoord_t') for 1st argument

    explicit cell_level_layout(const typename ClockedLayout::aspect_ratio& ar = {}, const std::string& name = "",
             ^

include/fiction/layouts/cell_level_layout.hpp:112: candidate constructor not viable: no known conversion from 'decltype(this->max.x())' (aka 'unsigned long') to 'const typename clocked_layout<cartesian_layout<>>::aspect_ratio' (aka 'const aspect_ratiofiction::offset::ucoord_t') for 1st argument

    cell_level_layout(const typename ClockedLayout::aspect_ratio& ar, const clocking_scheme<cell>& scheme,
    ^

include/fiction/layouts/cell_level_layout.hpp:124: candidate constructor not viable: requires single argument 's', but 3 arguments were provided

    explicit cell_level_layout(std::shared_ptr<cell_level_layout_storage<cell>> s) : strg{std::move(s)} {}
             ^

include/fiction/layouts/cell_level_layout.hpp:130: candidate constructor not viable: requires single argument 'lyt', but 3 arguments were provided

    explicit cell_level_layout(const ClockedLayout& lyt) :
             ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided

class cell_level_layout : public ClockedLayout
      ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 3 were provided

class cell_level_layout : public ClockedLayout
      ^

* @param y The y-coordinate value.
*/
template <typename X, typename Y>
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>' [clang-diagnostic-error]

    aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
                             ^
Additional context

include/fiction/algorithms/physical_design/apply_gate_library.hpp:235: in instantiation of function template specialization 'fiction::aspect_ratio<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>::aspect_ratio<unsigned long, unsigned long>' requested here

        return {std::max(max_coord_even_y.x, max_coord_odd_y.x), std::max(max_coord_even_x.y, max_coord_odd_x.y)};
               ^

include/fiction/algorithms/physical_design/apply_gate_library.hpp:43: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<>>>>>::determine_aspect_ratio_for_cell_level_layout' requested here

            cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
                     ^

include/fiction/algorithms/physical_design/apply_gate_library.hpp:268: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<>>>>>::apply_gate_library_impl' requested here

    detail::apply_gate_library_impl<CellLyt, GateLibrary, GateLyt> p{lyt};
                                                                   ^

test/io/write_sqd_layout.cpp:175: in instantiation of function template specialization 'fiction::apply_gate_library<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<>>>>>' requested here

    const auto c_layout = apply_gate_library<sidb_layout, sidb_bestagon_library>(g_layout);
                          ^

include/fiction/layouts/cell_level_layout.hpp:95: candidate constructor not viable: no known conversion from 'int' to 'const typename clocked_layout<cartesian_layout<>>::aspect_ratio' (aka 'const aspect_ratiofiction::offset::ucoord_t') for 1st argument

    explicit cell_level_layout(const typename ClockedLayout::aspect_ratio& ar = {}, const std::string& name = "",
             ^

include/fiction/layouts/cell_level_layout.hpp:112: candidate constructor not viable: no known conversion from 'int' to 'const typename clocked_layout<cartesian_layout<>>::aspect_ratio' (aka 'const aspect_ratiofiction::offset::ucoord_t') for 1st argument

    cell_level_layout(const typename ClockedLayout::aspect_ratio& ar, const clocking_scheme<cell>& scheme,
    ^

include/fiction/layouts/cell_level_layout.hpp:124: candidate constructor not viable: requires single argument 's', but 3 arguments were provided

    explicit cell_level_layout(std::shared_ptr<cell_level_layout_storage<cell>> s) : strg{std::move(s)} {}
             ^

include/fiction/layouts/cell_level_layout.hpp:130: candidate constructor not viable: requires single argument 'lyt', but 3 arguments were provided

    explicit cell_level_layout(const ClockedLayout& lyt) :
             ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided

class cell_level_layout : public ClockedLayout
      ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 3 were provided

class cell_level_layout : public ClockedLayout
      ^

* @param y The y-coordinate value.
*/
template <typename X, typename Y>
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>' [clang-diagnostic-error]

    aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
                             ^
Additional context

include/fiction/algorithms/physical_design/apply_gate_library.hpp:235: in instantiation of function template specialization 'fiction::aspect_ratio<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>::aspect_ratio<unsigned long, unsigned long>' requested here

        return {std::max(max_coord_even_y.x, max_coord_odd_y.x), std::max(max_coord_even_x.y, max_coord_odd_x.y)};
               ^

include/fiction/algorithms/physical_design/apply_gate_library.hpp:43: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>::determine_aspect_ratio_for_cell_level_layout' requested here

            cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
                     ^

include/fiction/algorithms/physical_design/apply_gate_library.hpp:268: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>::apply_gate_library_impl' requested here

    detail::apply_gate_library_impl<CellLyt, GateLibrary, GateLyt> p{lyt};
                                                                   ^

test/io/print_layout.cpp:750: in instantiation of function template specialization 'fiction::apply_gate_library<fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>' requested here

ll_layout_or       = apply_gate_library<sidb_cell_clk_lyt, sidb_bestagon_library>(layout);
                     ^

include/fiction/layouts/cell_level_layout.hpp:95: candidate constructor not viable: no known conversion from 'int' to 'const typename clocked_layout<cartesian_layout<>>::aspect_ratio' (aka 'const aspect_ratiofiction::offset::ucoord_t') for 1st argument

    explicit cell_level_layout(const typename ClockedLayout::aspect_ratio& ar = {}, const std::string& name = "",
             ^

include/fiction/layouts/cell_level_layout.hpp:112: candidate constructor not viable: no known conversion from 'int' to 'const typename clocked_layout<cartesian_layout<>>::aspect_ratio' (aka 'const aspect_ratiofiction::offset::ucoord_t') for 1st argument

    cell_level_layout(const typename ClockedLayout::aspect_ratio& ar, const clocking_scheme<cell>& scheme,
    ^

include/fiction/layouts/cell_level_layout.hpp:124: candidate constructor not viable: requires single argument 's', but 3 arguments were provided

    explicit cell_level_layout(std::shared_ptr<cell_level_layout_storage<cell>> s) : strg{std::move(s)} {}
             ^

include/fiction/layouts/cell_level_layout.hpp:130: candidate constructor not viable: requires single argument 'lyt', but 3 arguments were provided

    explicit cell_level_layout(const ClockedLayout& lyt) :
             ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided

class cell_level_layout : public ClockedLayout
      ^

include/fiction/layouts/cell_level_layout.hpp:49: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 3 were provided

class cell_level_layout : public ClockedLayout
      ^

* @param y The y-coordinate value.
*/
template <typename X, typename Y>
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>' [clang-diagnostic-error]

    aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
                                           ^
Additional context

include/fiction/technology/sidb_lattice.hpp:66: candidate constructor not viable: requires at most 2 arguments, but 3 were provided

    explicit sidb_lattice(const aspect_ratio_t<Lyt>& ar, const std::string& name = "") : Lyt(ar, name)
             ^

include/fiction/technology/sidb_lattice.hpp:55: candidate constructor not viable: requires single argument 'layout', but 3 arguments were provided

    explicit sidb_lattice(const Lyt& layout) : Lyt(layout)
             ^

include/fiction/technology/sidb_lattice.hpp:37: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided

class sidb_lattice<LatticeOrientation, Lyt, false> : public Lyt
      ^

include/fiction/technology/sidb_lattice.hpp:37: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 3 were provided

class sidb_lattice<LatticeOrientation, Lyt, false> : public Lyt
      ^

include/fiction/technology/sidb_lattice.hpp:45: candidate constructor not viable: requires 0 arguments, but 3 were provided

    explicit sidb_lattice() : Lyt()
             ^

* @param y The y-coordinate value.
*/
template <typename X, typename Y>
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching constructor for initialization of 'fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>' [clang-diagnostic-error]

    aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
                             ^
Additional context

include/fiction/algorithms/physical_design/apply_gate_library.hpp:235: in instantiation of function template specialization 'fiction::aspect_ratio<fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>>::aspect_ratio<unsigned long, unsigned long>' requested here

        return {std::max(max_coord_even_y.x, max_coord_odd_y.x), std::max(max_coord_even_x.y, max_coord_odd_x.y)};
               ^

include/fiction/algorithms/physical_design/apply_gate_library.hpp:43: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>::determine_aspect_ratio_for_cell_level_layout' requested here

            cell_lyt{determine_aspect_ratio_for_cell_level_layout(gate_lyt)}
                     ^

include/fiction/algorithms/physical_design/apply_gate_library.hpp:268: in instantiation of member function 'fiction::detail::apply_gate_library_impl<fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>::apply_gate_library_impl' requested here

    detail::apply_gate_library_impl<CellLyt, GateLibrary, GateLyt> p{lyt};
                                                                   ^

test/io/print_layout.cpp:277: in instantiation of function template specialization 'fiction::apply_gate_library<fiction::sidb_lattice<fiction::sidb_100_lattice, fiction::cell_level_layout<fiction::sidb_technology, fiction::clocked_layout<fiction::cartesian_layout<>>>>, fiction::sidb_bestagon_library, fiction::gate_level_layout<fiction::clocked_layout<fiction::tile_based_layout<fiction::hexagonal_layout<fiction::offset::ucoord_t, fiction::odd_row_hex>>>>>' requested here

layout.create_or({}, {}, {0, 0});
                                                                                                    ^

include/fiction/technology/sidb_lattice.hpp:66: candidate constructor not viable: requires at most 2 arguments, but 3 were provided

    explicit sidb_lattice(const aspect_ratio_t<Lyt>& ar, const std::string& name = "") : Lyt(ar, name)
             ^

include/fiction/technology/sidb_lattice.hpp:55: candidate constructor not viable: requires single argument 'layout', but 3 arguments were provided

    explicit sidb_lattice(const Lyt& layout) : Lyt(layout)
             ^

include/fiction/technology/sidb_lattice.hpp:37: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided

class sidb_lattice<LatticeOrientation, Lyt, false> : public Lyt
      ^

include/fiction/technology/sidb_lattice.hpp:37: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 3 were provided

class sidb_lattice<LatticeOrientation, Lyt, false> : public Lyt
      ^

include/fiction/technology/sidb_lattice.hpp:45: candidate constructor not viable: requires 0 arguments, but 3 were provided

    explicit sidb_lattice() : Lyt()
             ^

* @param y The y-coordinate value.
*/
template <typename X, typename Y>
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: reference to non-static member function must be called; did you mean to call it with no arguments? [clang-diagnostic-error]

Suggested change
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x())>(x), static_cast<decltype(max.y)>(y), 0}

* @param y The y-coordinate value.
*/
template <typename X, typename Y>
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: reference to non-static member function must be called; did you mean to call it with no arguments? [clang-diagnostic-error]

Suggested change
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y)>(y), 0}
aspect_ratio(X x, Y y) : min{0, 0, 0}, max{static_cast<decltype(max.x)>(x), static_cast<decltype(max.y())>(y), 0}

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -227,6 +221,43 @@ inline void coordinate_utility(pybind11::module& m)
DOC(fiction_siqad_to_siqad_coord));
}

template <typename CoordinateType>
inline void aspect_ratio_bindings(pybind11::module& m, const std::string& coordinate_type)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "std::string" is directly included [misc-include-cleaner]

bindings/mnt/pyfiction/include/pyfiction/layouts/coordinates.hpp:17:

+ #include <string>

@@ -294,7 +294,7 @@ template <typename HexLyt, typename CartLyt>
const auto mapped_layout_width = layout_max.x;
const auto mapped_layout_height = layout_max.y;

hex_layout.resize({mapped_layout_width, mapped_layout_height, hex_layout.z()});
hex_layout.resize(aspect_ratio<coordinate<CartLyt>>{mapped_layout_width, mapped_layout_height, hex_layout.z()});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::aspect_ratio" is directly included [misc-include-cleaner]

include/fiction/algorithms/physical_design/hexagonalization.hpp:7:

- #pragma GCC diagnostic push
+ #include "fiction/layouts/coordinates.hpp"
+ #pragma GCC diagnostic push

@@ -224,7 +224,7 @@ class read_fqca_layout_impl
}

// resize the layout to fit all cells
lyt.resize(max_cell_pos);
lyt.resize(aspect_ratio{max_cell_pos});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::aspect_ratio" is directly included [misc-include-cleaner]

include/fiction/io/read_fqca_layout.hpp:7:

- #include "fiction/technology/cell_technologies.hpp"
+ #include "fiction/layouts/coordinates.hpp"
+ #include "fiction/technology/cell_technologies.hpp"

@@ -147,16 +147,23 @@ class read_sidb_surface_defects_impl
max_cell_pos.y = static_cast<decltype(max_cell_pos.y)>(matrix_matches.empty() ? 0 : matrix_matches.size() - 1);

// resize the layout to fit all surface defects
lyt.resize(max_cell_pos);
lyt.resize(aspect_ratio{max_cell_pos});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::aspect_ratio" is directly included [misc-include-cleaner]

include/fiction/io/read_sidb_surface_defects.hpp:7:

- #include "fiction/technology/sidb_defect_surface.hpp"
+ #include "fiction/layouts/coordinates.hpp"
+ #include "fiction/technology/sidb_defect_surface.hpp"

@@ -144,7 +144,7 @@ class read_sqd_layout_impl
}

// resize the layout to fit all cells
lyt.resize(max_cell_pos);
lyt.resize(aspect_ratio{max_cell_pos});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::aspect_ratio" is directly included [misc-include-cleaner]

include/fiction/io/read_sqd_layout.hpp:7:

- #include "fiction/technology/cell_technologies.hpp"
+ #include "fiction/layouts/coordinates.hpp"
+ #include "fiction/technology/cell_technologies.hpp"

@@ -102,7 +102,7 @@ TEST_CASE("Deep copy shifted Cartesian layout", "[shifted-cartesian-layout]")

auto copy = original.clone();

copy.resize({10, 10, 1});
copy.resize(aspect_ratio<coordinate<decltype(original)>>{10, 10, 1});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::aspect_ratio" is directly included [misc-include-cleaner]

    copy.resize(aspect_ratio<coordinate<decltype(original)>>{10, 10, 1});
                ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -36,7 +36,7 @@ TEST_CASE("Draw empty Cartesian layout", "[dot-drawers]")
{
using gate_layout = gate_level_layout<clocked_layout<tile_based_layout<cartesian_layout<offset::ucoord_t>>>>;

gate_layout layout{gate_layout::aspect_ratio{2, 2}};
gate_layout layout{{2, 2}};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'layout' of type 'gate_layout' (aka 'gate_level_layout<clocked_layout<tile_based_layout<cartesian_layoutoffset::ucoord_t>>>') can be declared 'const' [misc-const-correctness]

Suggested change
gate_layout layout{{2, 2}};
gate_layout const layout{{2, 2}};

@@ -221,7 +221,7 @@ TEST_CASE("Draw empty hexagonal layouts", "[dot-drawers]")
using gate_layout =
gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, odd_row_hex>>>>;

gate_layout layout{gate_layout::aspect_ratio{2, 2}};
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'layout' of type 'gate_layout' (aka 'gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, odd_row_hex>>>>') can be declared 'const' [misc-const-correctness]

Suggested change
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
gate_layout const layout{gate_layout::aspect_ratio_type{2, 2}};

@@ -271,7 +271,7 @@ TEST_CASE("Draw empty hexagonal layouts", "[dot-drawers]")
using gate_layout =
gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, even_row_hex>>>>;

gate_layout layout{gate_layout::aspect_ratio{2, 2}};
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'layout' of type 'gate_layout' (aka 'gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, even_row_hex>>>>') can be declared 'const' [misc-const-correctness]

Suggested change
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
gate_layout const layout{gate_layout::aspect_ratio_type{2, 2}};

@@ -322,7 +322,7 @@ TEST_CASE("Draw empty hexagonal layouts", "[dot-drawers]")
using gate_layout =
gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, odd_column_hex>>>>;

gate_layout layout{gate_layout::aspect_ratio{2, 2}};
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'layout' of type 'gate_layout' (aka 'gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, odd_column_hex>>>>') can be declared 'const' [misc-const-correctness]

Suggested change
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
gate_layout const layout{gate_layout::aspect_ratio_type{2, 2}};

@@ -372,7 +372,7 @@ TEST_CASE("Draw empty hexagonal layouts", "[dot-drawers]")
using gate_layout =
gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, even_column_hex>>>>;

gate_layout layout{gate_layout::aspect_ratio{2, 2}};
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'layout' of type 'gate_layout' (aka 'gate_level_layout<clocked_layout<tile_based_layout<hexagonal_layout<offset::ucoord_t, even_column_hex>>>>') can be declared 'const' [misc-const-correctness]

Suggested change
gate_layout layout{gate_layout::aspect_ratio_type{2, 2}};
gate_layout const layout{gate_layout::aspect_ratio_type{2, 2}};

@@ -103,13 +103,13 @@ TEST_CASE("Cartesian coordinate iteration", "[cartesian-layout]")

visited.clear();

cartesian_layout<offset::ucoord_t>::aspect_ratio ar_ground{ar.x, ar.y, 0};
aspect_ratio<offset::ucoord_t> ar_ground{ar.x(), ar.y(), 0};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::aspect_ratio" is directly included [misc-include-cleaner]

    aspect_ratio<offset::ucoord_t> ar_ground{ar.x(), ar.y(), 0};
    ^

@@ -36,13 +36,13 @@ TEST_CASE("Deep copy gate-level layout", "[gate-level-layout]")
{
using gate_layout = gate_level_layout<clocked_layout<tile_based_layout<cartesian_layout<offset::ucoord_t>>>>;

gate_layout original{gate_layout::aspect_ratio{5, 5, 0}, twoddwave_clocking<gate_layout>(), "Original"};
gate_layout original{{5, 5, 0}, twoddwave_clocking<gate_layout>(), "Original"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::twoddwave_clocking" is directly included [misc-include-cleaner]

test/layouts/gate_level_layout.cpp:6:

- #include "utils/blueprints/layout_blueprints.hpp"
+ #include "fiction/layouts/clocking_scheme.hpp"
+ #include "utils/blueprints/layout_blueprints.hpp"

@@ -516,7 +516,7 @@ TEST_CASE("compute functions from AND and NOT gates", "[gate-level-layout]")

REQUIRE(mockturtle::has_compute_v<gate_layout, kitty::dynamic_truth_table>);

gate_layout layout{gate_layout::aspect_ratio{3, 1, 0}, open_clocking<gate_layout>(num_clks::FOUR)};
gate_layout layout{{3, 1, 0}, open_clocking<gate_layout>(num_clks::FOUR)};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::num_clks" is directly included [misc-include-cleaner]

    gate_layout layout{{3, 1, 0}, open_clocking<gate_layout>(num_clks::FOUR)};
                                                             ^

@@ -516,7 +516,7 @@ TEST_CASE("compute functions from AND and NOT gates", "[gate-level-layout]")

REQUIRE(mockturtle::has_compute_v<gate_layout, kitty::dynamic_truth_table>);

gate_layout layout{gate_layout::aspect_ratio{3, 1, 0}, open_clocking<gate_layout>(num_clks::FOUR)};
gate_layout layout{{3, 1, 0}, open_clocking<gate_layout>(num_clks::FOUR)};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "fiction::open_clocking" is directly included [misc-include-cleaner]

    gate_layout layout{{3, 1, 0}, open_clocking<gate_layout>(num_clks::FOUR)};
                                  ^

@Drewniok
Copy link
Collaborator

Drewniok commented Feb 5, 2025

@simon1hofmann I have fixed most of the issues. Still, there are a few issues we have to solve. Please let me know when you need help. I try to help as much as I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants