qiskit-community / qiskit-optimization

Quantum Optimization
https://qiskit-community.github.io/qiskit-optimization/
Apache License 2.0
230 stars 141 forks source link

Fix bug in max-cut solver (issue #611) #635

Closed TolisChal closed 3 weeks ago

TolisChal commented 1 month ago

This PR fixes the bug in the max-cut solver as reported in https://github.com/qiskit-community/qiskit-optimization/issues/611

TolisChal commented 1 month ago

Hi @t-imamichi, @woodsp-ibm, @stefan-woerner! I take care of the pylint errors in this PR: https://github.com/qiskit-community/qiskit-optimization/pull/634 Shall I wait for that one to be merged and rebase this one?

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 11496393782

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_optimization/applications/max_cut.py 0 2 0.0%
<!-- Total: 27 29 93.1% -->
Totals Coverage Status
Change from base Build 10312432197: 0.0%
Covered Lines: 4446
Relevant Lines: 4788

💛 - Coveralls
t-imamichi commented 1 month ago

Thank you for fixing an issue. I read the changelog of pylint and found that a new check too-many-positional-arguments was introduced by pylint 3.3. https://pylint.readthedocs.io/en/latest/whatsnew/3/3.3/index.html#new-checks

Because qiskit-optimization already has the following config for max-args, how about adding a similar config for max-positional-args as follows too? https://github.com/qiskit-community/qiskit-optimization/blob/807d48167caf11cd93bec85f26b34614fb7868da/.pylintrc#L313

max-positional-arguments=8

https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/too-many-positional-arguments.html

t-imamichi commented 1 month ago

Let me try the config #637

t-imamichi commented 1 month ago

How about merging #637? Could you take a look at it? You can minimize the change with the config.

t-imamichi commented 1 month ago

I closed #637 following @woodsp-ibm's comment.

t-imamichi commented 1 month ago

LGTM overall. I wrote some comments.