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

✨♻️ Project Setup and Structural Improvements #7

Merged
merged 22 commits into from
Apr 21, 2024

Conversation

burgholzer
Copy link
Member

@burgholzer burgholzer commented Apr 19, 2024

Disclaimer: This is a huge huge PR. This is not how things should be done typically, but it is close to impossible to cleanly separate the changes here.

This PR does:

  • vendor mqt-misim as part of mqt-qudits. this includes all changes from ✨♻️ MQT Project Setup mqt-misim#23
  • properly setting up Python packaging based on scikit-build-core
  • update all pre-commit checks and add new ones related to the Python part of the project
  • adds a dependabot configuration to keep dependencies up to date
  • adds a very basic ReadTheDocs documentation setup (@KevinMTO please extend this in the future whenever you have time)
  • adopts the MQT reusable CI workflows that take care of everything from testing to coverage to linting to packaging
  • adds a release drafter configuration for automating release management

In addition to the above, the PR tries to cleanup the repository a little bit and bring some structure into the general chaos. Even after this refactor, the code still requires quite some work.
Ruff currently throws > 800 errors, mypy throws >600, clang-tidy throws 40 warnings, and CodeQL also isn't too happy.
Due to the lack of tests, I am also pretty sure that there are quite some mistakes throughout the code.

Despite the above, I would propose to merge this PR as quickly as possible and continue the development, fixing issues, adding typing, and restructuring the library based on the merged PR.

@burgholzer burgholzer added documentation Improvements or additions to documentation enhancement New feature or request dependencies Pull requests that update a dependency file usability minor continuous integration feature c++ python labels Apr 19, 2024
@burgholzer burgholzer self-assigned this Apr 19, 2024
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

backend_version: str | None = None,
**fields: Any,
) -> None:
self._options = self._default_options()

Check warning

Code scanning / CodeQL

`__init__` method calls overridden method Warning

Call to self.
_default_options
in __init__ method, which is overridden by
method FakeIonTraps2Six._default_options
.
Call to self.
_default_options
in __init__ method, which is overridden by
method FakeIonTraps2Trits._default_options
.
tol = Optimizer.OBJ_FIDELITY

best_layer, best_error, best_xi = (low + (high - low) // 2, np.inf, [])
mid, error, xi = (low + (high - low) // 2, np.inf, [])

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'mid' is unnecessary as it is
redefined
before this value is used.
tol = Optimizer.OBJ_FIDELITY

best_layer, best_error, best_xi = (low + (high - low) // 2, np.inf, [])
mid, error, xi = (low + (high - low) // 2, np.inf, [])

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'error' is unnecessary as it is
redefined
before this value is used.
tol = Optimizer.OBJ_FIDELITY

best_layer, best_error, best_xi = (low + (high - low) // 2, np.inf, [])
mid, error, xi = (low + (high - low) // 2, np.inf, [])

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'xi' is unnecessary as it is
redefined
before this value is used.
Comment on lines +33 to +36
# for field in fields:
# if field not in self._options.data:
# msg = f"Options field '{field}' is not valid for this backend"
# raise AttributeError(msg)

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
include/dd/ComplexTable.hpp Fixed Show fixed Hide fixed
include/dd/Definitions.hpp Fixed Show fixed Hide fixed
include/dd/Definitions.hpp Fixed Show fixed Hide fixed
include/dd/MDDPackage.hpp Fixed Show fixed Hide fixed
include/dd/MDDPackage.hpp Fixed Show fixed Hide fixed
include/dd/Definitions.hpp Fixed Show fixed Hide fixed
test/test_pkg.cpp Fixed Show fixed Hide fixed
include/dd/UniqueTable.hpp Fixed Show fixed Hide fixed
include/dd/MDDPackage.hpp Fixed Show fixed Hide fixed
test/test_pkg.cpp Fixed Show fixed Hide fixed
Copy link
Contributor

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-tidy reports: 40 concern(s)
  • include/dd/Complex.hpp:20:8: warning: [cppcoreguidelines-pro-type-member-init]

    constructor does not initialize these fields: real, img

    struct Complex {
           ^
  • include/dd/Complex.hpp:75:25: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'zero' is non-const and globally accessible, consider making it const

    inline Complex Complex::zero{&ComplexTable<>::zero, &ComplexTable<>::zero};
                            ^
  • include/dd/Complex.hpp:78:25: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'one' is non-const and globally accessible, consider making it const

    inline Complex Complex::one{&ComplexTable<>::one, &ComplexTable<>::zero};
                            ^
  • include/dd/ComplexCache.hpp:25:3: warning: [cppcoreguidelines-pro-type-member-init]

    constructor does not initialize these fields: chunkIt, chunkEndIt

      ComplexCache() : allocationSize(INITIAL_ALLOCATION_SIZE) {
      ^
  • include/dd/ComplexNumbers.hpp:33:31: warning: [misc-unused-parameters]

    parameter 'tol' is unused

      static void setTolerance(fp tol) { ComplexTable<>::setTolerance(tol); }
                                  ^~~
                                   /*tol*/
  • include/dd/ComplexNumbers.hpp:33:67: warning: [cppcoreguidelines-init-variables]

    variable 'tol' is not initialized

      static void setTolerance(fp tol) { ComplexTable<>::setTolerance(tol); }
                                                                      ^
                                                                          = 0
  • include/dd/ComplexNumbers.hpp:144:11: warning: [readability-convert-member-functions-to-static]

    method 'lookup' can be made static

      Complex lookup(const Complex& c) {
              ^
      static 
  • include/dd/ComplexNumbers.hpp:156:11: warning: [readability-convert-member-functions-to-static]

    method 'lookup' can be made static

      Complex lookup(const fp& r, const fp& i) {
              ^
      static 
  • include/dd/ComplexNumbers.hpp:164:7: warning: [bugprone-branch-clone]

    if with identical then and else branches

          if (absr < decltype(complexTable)::tolerance()) {
          ^
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/ComplexNumbers.hpp:166:9: note: else branch starts here
          } else {
            ^
  • include/dd/ComplexNumbers.hpp:178:7: warning: [bugprone-branch-clone]

    if with identical then and else branches

          if (absi < decltype(complexTable)::tolerance()) {
          ^
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/ComplexNumbers.hpp:180:9: note: else branch starts here
          } else {
            ^
  • include/dd/ComplexNumbers.hpp:189:18: warning: [readability-convert-member-functions-to-static]

    method 'lookup' can be made static

      inline Complex lookup(const ComplexValue& c) { return lookup(c.r, c.i); }
                     ^
      static 
  • include/dd/ComplexNumbers.hpp:213:18: warning: [readability-convert-member-functions-to-static]

    method 'getTemporary' can be made static

      inline Complex getTemporary(const fp& r, const fp& i) {
                     ^
      static 
  • include/dd/ComplexNumbers.hpp:220:18: warning: [readability-convert-member-functions-to-static]

    method 'getTemporary' can be made static

      inline Complex getTemporary(const ComplexValue& c) {
                     ^
      static 
  • include/dd/ComplexNumbers.hpp:226:18: warning: [readability-convert-member-functions-to-static]

    method 'getCached' can be made static

      inline Complex getCached(const fp& r, const fp& i) {
                     ^
      static 
  • include/dd/ComplexNumbers.hpp:233:18: warning: [readability-convert-member-functions-to-static]

    method 'getCached' can be made static

      inline Complex getCached(const ComplexValue& c) {
                     ^
      static 
  • include/dd/ComplexTable.hpp:105:23: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'zero' is non-const and globally accessible, consider making it const

      static inline Entry zero{
                          ^
  • include/dd/ComplexTable.hpp:109:23: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'sqrt2_2' is non-const and globally accessible, consider making it const

      static inline Entry sqrt2_2{
                          ^
  • include/dd/ComplexTable.hpp:109:23: warning: [readability-identifier-naming]

    invalid case style for variable 'sqrt2_2'

      static inline Entry sqrt2_2{
                          ^~~~~~~
                          sqrt22
  • include/dd/ComplexTable.hpp:113:23: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'one' is non-const and globally accessible, consider making it const

      static inline Entry one{
                          ^
  • include/dd/ComplexTable.hpp:522:20: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'TOLERANCE' is non-const and globally accessible, consider making it const

      static inline fp TOLERANCE =
                       ^
  • include/dd/ComplexTable.hpp:522:20: warning: [readability-identifier-naming]

    invalid case style for variable 'TOLERANCE'

      static inline fp TOLERANCE =
                       ^~~~~~~~~
                       tolerance
  • include/dd/Edge.hpp:38:28: warning: [readability-identifier-naming]

    invalid case style for global constant 'one'

      static const inline Edge one{
                               ^~~
                               ONE
  • include/dd/Edge.hpp:42:28: warning: [readability-identifier-naming]

    invalid case style for global constant 'zero'

      static const inline Edge zero{
                               ^~~~
                               ZERO
  • include/dd/MDDPackage.hpp:58:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:52:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:118:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:52:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:127:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:52:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:140:29: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'terminal' provides global access to a non-const object; consider making the pointed-to data 'const'

        constexpr static vNode* terminal{
                                ^
  • include/dd/MDDPackage.hpp:140:29: warning: [readability-identifier-naming]

    invalid case style for global constant 'terminal'

        constexpr static vNode* terminal{
                                ^~~~~~~~
                                TERMINAL
  • include/dd/MDDPackage.hpp:360:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:52:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:373:29: warning: [cppcoreguidelines-avoid-non-const-global-variables]

    variable 'terminal' provides global access to a non-const object; consider making the pointed-to data 'const'

        constexpr static mNode* terminal{
                                ^
  • include/dd/MDDPackage.hpp:373:29: warning: [readability-identifier-naming]

    invalid case style for global constant 'terminal'

    note: this fix will not be applied because it overlaps with another fix
  • include/dd/MDDPackage.hpp:602:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:52:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:839:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:699:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:1543:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:1250:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:1562:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:1250:1: note: previously declared here
    public:
    ^
  • include/dd/MDDPackage.hpp:1949:1: warning: [readability-redundant-access-specifiers]

    redundant access specifier has the same accessibility as the previous access specifier

    public:
    ^~~~~~~
    /home/runner/work/mqt-qudits/mqt-qudits/include/dd/MDDPackage.hpp:1866:1: note: previously declared here
    public:
    ^
  • include/dd/UniqueTable.hpp:103:11: warning: [cppcoreguidelines-init-variables]

    variable 'p' is not initialized

        Node* p = tables[static_cast<std::size_t>(v)][key];
              ^
                = nullptr
  • include/dd/UniqueTable.hpp:237:5: warning: [misc-const-correctness]

    variable 'collected' of type 'std::size_t' (aka 'unsigned long') can be declared 'const'

        std::size_t collected = 0;
        ^
                    const 
  • include/dd/UniqueTable.hpp:238:5: warning: [misc-const-correctness]

    variable 'remaining' of type 'std::size_t' (aka 'unsigned long') can be declared 'const'

        std::size_t remaining = 0;
        ^
                    const 
  • test/main.cpp:7:3: warning: [misc-const-correctness]

    variable 'numLines' of type 'dd::QuantumRegisterCount' (aka 'unsigned char') can be declared 'const'

      dd::QuantumRegisterCount numLines = 2U;
      ^
                               const 

Have any feedback or feature suggestions? Share it here.

@burgholzer
Copy link
Member Author

Ok. At least the C++ part from MiSiM works as intended.
Most of the clang-tidy warnings should be relatively straight-forward to fix.

The python part is still failing left and right. But I guess that's all part of the process.
At the moment, I cannot dedicate more time to this.
The general infrastructure is all set up and should allow you, @KevinMTO, to focus on the functionality itself.

@KevinMTO KevinMTO enabled auto-merge (rebase) April 21, 2024 10:43
@KevinMTO KevinMTO disabled auto-merge April 21, 2024 10:44
@burgholzer burgholzer merged commit 423fa18 into main Apr 21, 2024
15 of 33 checks passed
@burgholzer burgholzer deleted the improve-project-setup branch April 21, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ continuous integration dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request feature minor python usability
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants