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

Fix _minpoly_groebner() to work with generic AlgebraicNumber's #433

merged 5 commits into from Feb 17, 2017


1 participant
Copy link

skirpichev commented Feb 15, 2017

  • more tests?
  • 100% coverage
  • rebase & cleanup

@skirpichev skirpichev added this to the 0.9.0 milestone Feb 15, 2017

@skirpichev skirpichev added the testing label Feb 16, 2017

@skirpichev skirpichev force-pushed the skirpichev:fix-431 branch 2 times, most recently from 13bf89a to e890a49 Feb 17, 2017

skirpichev added some commits Feb 15, 2017

Fix _minpoly_groebner() to work with generic AlgebraicNumber's
Previously, only case with default coeffs == (1, 0)
was covered.  This closes #431.

Only linear cases works currently, i.e. no more than
two coeffs for AlgebraicNumber, otherwise PolynomialError raised:

    In [1]: r = RootOf(x**7 + 3*x - 1, 3)

    In [2]: a = AlgebraicNumber(r, (1, 2, 3, 0, 1))

    In [3]: minimal_polynomial(a, x)
    KeyError                                  Traceback (most recent call last)
    diofant/polys/ in _parallel_dict_from_expr_if_gens(exprs, opt)
    --> 212                         monom[indices[base]] += exp
        213                     except KeyError:

    KeyError: Pow(Dummy('a3'), Rational(1, 4))

    During handling of the above exception, another exception occurred:

    diofant/polys/ in _parallel_dict_from_expr_if_gens(exprs, opt)
        215                             coeff.append(factor)
        216                         else:
    --> 217                             raise PolynomialError("%s contains an element of the generators set" % factor)
        219             monom = tuple(monom)

    PolynomialError: _a3**(1/4) contains an element of the generators set

At least, there is no more wrong answer.

Also do some cleanup of bottom_up_scan() helper.
More cleanup for _minpoly_groebner()
Improve code coverage with new tests.

@skirpichev skirpichev force-pushed the skirpichev:fix-431 branch from e890a49 to 7083c4c Feb 17, 2017

@skirpichev skirpichev merged commit 62dd0aa into diofant:master Feb 17, 2017

3 checks passed

codecov/patch 100% of diff hit (target 97%)
codecov/project Absolute coverage decreased by -<1% but relative coverage increased by +8% compared to fda3b19
continuous-integration/travis-ci/pr The Travis CI build passed

@skirpichev skirpichev deleted the skirpichev:fix-431 branch Feb 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment