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

Added functionality and documentation for calculating roots of univariate polynomials #935

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

monien
Copy link
Contributor

@monien monien commented Apr 11, 2021

Added functionality and documentation for calculating roots of univariate polynomials over the integers (fmpz_poly_t) over finite fields (fq_t), p-adic fields (padic_t) and q-adic fields (qadic_t).

@fredrik-johansson
Copy link
Collaborator

What is the rationale for making this a new module instead of putting root-finding methods in the fq, padic and qadic modules?

@monien
Copy link
Contributor Author

monien commented Apr 12, 2021 via email

@monien
Copy link
Contributor Author

monien commented Apr 12, 2021 via email

@wbhart
Copy link
Collaborator

wbhart commented Apr 12, 2021

These are finding padic, qadic or finite field roots of an integer polynomial (Z[x]). Are you sure you want it that way around @fredrik-johansson ?

@fredrik-johansson
Copy link
Collaborator

Well, someone who comes across this module might be disappointed that it doesn't do roots in R and C :-)

Does @tthsqe12 have an opinion on the interface?

@tthsqe12
Copy link
Contributor

I don't have much of an opinion on the interface. But I do have questions on the code. First of all, there is no test code yet, so we cannot be sure what the functions are supposed to do. The documentation is not specific on what roots it finds and to what precision, ect.
Here is what I have found so far: an example that words, a mundane crash, and weird root output (the root is listed as multiple and is only correct to precision 2, and the other root is missed):

#include <stdio.h>
#include <stdlib.h>
#include <gmp.h>
#include "flint.h"
#include "fmpz.h"
#include "fmpz_poly.h"
#include "ulong_extras.h"
#include "padic.h"
#include "fmpz_poly_roots.h"

int
main(void)
{
    int i, result;
    FLINT_TEST_INIT(state);

    flint_printf("roots....\n");
    fflush(stdout);

    {
        fmpz_poly_t a, b;
        fmpz_t p;
        padic_ctx_t Q5;
        fmpz_poly_roots_padic_t roots;

        fmpz_init_set_ui(p, 5);
        padic_ctx_init(Q5, p, 20, 20, PADIC_SERIES);
        fmpz_poly_init(a);

        flint_printf("------- roots works ---------\n");
        fmpz_poly_set_str(a, "3  3 1 1");
       
        flint_printf("a: ");
        fmpz_poly_print_pretty(a, "x");
        flint_printf("\n");

        fmpz_poly_roots_padic_init2(roots, 2);
        fmpz_poly_roots_padic(roots, a, Q5);
        fmpz_poly_roots_padic_print(roots, Q5);
        fmpz_poly_roots_padic_clear(roots);

        flint_printf("------- roots crashes ---------\n");
        fmpz_poly_set_str(a, "3  -30 1 1");
       
        flint_printf("a: ");
        fmpz_poly_print_pretty(a, "x");
        flint_printf("\n");

        fmpz_poly_roots_padic_init2(roots, 2);
        /* this crashes because an fq root is zero so its 'coeffs' cannot be dereferenced */
        /*fmpz_poly_roots_padic(roots, a, Q5);
        flint_printf("roots:\n");
        fmpz_poly_roots_padic_print(roots, Q5);*/
        fmpz_poly_roots_padic_clear(roots);

        flint_printf("------- roots gives wrong answer? ---------\n");
        fmpz_poly_set_str(a, "3  6 -7 1");
       
        flint_printf("a: ");
        fmpz_poly_print_pretty(a, "x");
        flint_printf("\n");

        fmpz_poly_roots_padic_init2(roots, 2);
        fmpz_poly_roots_padic(roots, a, Q5);
        flint_printf("roots:\n");
        fmpz_poly_roots_padic_print(roots, Q5);
        fmpz_poly_roots_padic_clear(roots);

        flint_printf("roots mod 5^20 reported by fmpz_mod_poly_roots:\n");

        {
            slong i;
            fmpz_mod_ctx_t mod55;
            fmpz_mod_poly_t f;
            fmpz_mod_poly_factor_t fac;
            fmpz_t t;
            fmpz_factor_t fac55;

            fmpz_init(t);
            fmpz_pow_ui(t, p, 20);
            fmpz_mod_ctx_init(mod55, t);
            fmpz_mod_poly_init(f, mod55);
            fmpz_mod_poly_factor_init(fac, mod55);
            fmpz_factor_init(fac55);
            fmpz_factor(fac55, fmpz_mod_ctx_modulus(mod55));

            fmpz_mod_poly_set_fmpz_poly(f, a, mod55);
            fmpz_mod_poly_roots_factored(fac, f, 1, fac55, mod55);

            for (i = 0; i < fac->num; i++)
            {
                fmpz_mod_neg(t, fac->poly[i].coeffs + 0, mod55);
                fmpz_print(t);
                flint_printf(" %wd\n", fac->exp[i]);
            }

            fmpz_factor_clear(fac55);
            fmpz_mod_poly_factor_clear(fac, mod55);
            fmpz_mod_poly_clear(f, mod55);
            fmpz_mod_ctx_clear(mod55);
            fmpz_clear(t);
        }

        fmpz_poly_clear(a);
        padic_ctx_clear(Q5);
        fmpz_clear(p);
    }

    FLINT_TEST_CLEANUP(state);
    
    flint_printf("PASS\n");
    return 0;
}

with output:

------- roots works ---------
a: x^2+x+3
3 + 1*5^1 + 1*5^2 + 3*5^3 + 4*5^4 + 4*5^5 + 3*5^7 + 1*5^8 + 1*5^9 + 4*5^11 + 2*5^13 + 1*5^15 + 1*5^17 + 4*5^18 + 1*5^19 1
1 + 3*5^1 + 3*5^2 + 1*5^3 + 4*5^6 + 1*5^7 + 3*5^8 + 3*5^9 + 4*5^10 + 4*5^12 + 2*5^13 + 4*5^14 + 3*5^15 + 4*5^16 + 3*5^17 + 3*5^19 1
------- roots crashes ---------
a: x^2+x-30
------- roots gives wrong answer? ---------
a: x^2-7*x+6
roots:
1 + 3*5^1 + 2*5^2 + 2*5^3 + 2*5^4 + 2*5^5 + 2*5^6 + 2*5^7 + 2*5^8 + 2*5^9 + 2*5^10 + 2*5^11 + 2*5^12 + 2*5^13 + 2*5^14 + 2*5^15 + 2*5^16 + 2*5^17 + 2*5^18 + 2*5^19 2
roots mod 5^20 reported by fmpz_mod_poly_roots:
1 1
19073486328126 1
38146972656251 1
57220458984376 1
76293945312501 1
6 1
19073486328131 1
38146972656256 1
57220458984381 1
76293945312506 1
PASS

@tthsqe12
Copy link
Contributor

maybe @thofma has an opinion on how this kind of stuff is supposed to work

@tthsqe12
Copy link
Contributor

So, the problem here for Qp is: the code finds roots mod p then attemps to lift them using newton's method on f or f', f'' ect if there are multiple roots mod p. The problem is this last case of multiple roots.

@tthsqe12
Copy link
Contributor

tthsqe12 commented Apr 13, 2021

Im sure that @fredrik-johansson would have an opinion here, and I am going to give mine:
The roots function should behave as if it calculated the roots exactly (with multiplicities) and then truncated these exact roots to the precision of the padic_ctx_t. This means in particular for Q5:

  1. The roots of (x-1)*(x-6) should reported as [[1 + O(5), 1], [1 + O(5), 1]] in precision 1 output and as [[1 + O(5^2), 1], [6 + O(5^2), 1]] in precision 2 output.
  2. I am not sure how the padic_t handles relative precision, but there should be two simple roots output for x^2+x-30 in any precision.
  3. 5x-1 should have one simple root in any precision.
  4. x^2-5 should have no roots (in any precision).
  5. For the root of x (i.e. zero), the particular precision model used by the padic_t, of which I know nothing, is probably relevent.

This makes the function harder to code, but has the benefit of giving the function a well-defined mathematical prescription. This also means the output of the function is a priori reliable.

@tthsqe12
Copy link
Contributor

tthsqe12 commented Apr 13, 2021

In order to accomplish this exact root stuff for f(x) in Z[x], we can assume:

  1. f(0) != 0
  2. f is squarefree over Qp by a squarefree factorization over Z
  3. The leading coefficient of f is not divisible by p by a rescale of f.

At this point each exact root of f is identified by the following data:

  1. An integer k > 0
  2. An x0 in Z/(p^k Z) with f(x0) = 0 in Z/(p^k Z) and f'(x0) != 0 in Z/(p^floor((k+1)/2) Z)

Furthermore, it easy to see when (x0, k) and some other (x0', k') represent the same root, so we can calculate the roots mod p^k such that f' has at most half valuation at each of these roots mod p^k, and then delete duplicates.

@monien
Copy link
Contributor Author

monien commented Apr 13, 2021 via email

@tthsqe12
Copy link
Contributor

tthsqe12 commented Apr 13, 2021

Here is my opinion on the module thingy. In other modules we have a T_poly_t and and T_poly_factor_t and we put the root finding code in the factor module and return the roots ri and multiplicities ei as a "factorization" prod_i (x-ri)^ei. This is not great but was done to cut down on the number of flint types that need to be wrapped in foreign languages. The overhead of each x-ri is usually negligible compared to the root finding time.
At the moment we do not have qadic_poly nor do we have {p|q}adic_poly_factor_t.
I do not imagine that the factoring functions {p|q}adic_poly_factor will/can ever be implemented. Therefore, we are left with factoring fmpz_poly's over {p|q}adic. This root finding stuff does degree one factors. If we ever allow factors of arbitrary degree (which seems likely in the near future), there will definitely have to be a {p|q}adic_poly_factor_t to hold the answer. If the root finder and the hypothetical general factorer use different return types, it will be inconsistent with the other modules.

long summary: I don't care where the files are placed, but maybe the return type use for fmpz_poly -> {p|q}adic roots should be general enough to hold arbitrary degree factorizations to consistently support future extensions to the functions. Or, maybe roots in this case are a special enough case to warrant a special type... Arb has arb_fmpz_poly_complex_roots which takes squarefree exact input and just a raw pointer to the output. I don't think this is a great idea here because we don't know the number of roots in advance...

@fredrik-johansson
Copy link
Collaborator

Life will maybe become easier if we introduce non-underscore vector types with length management (fmpz_vec_t, etc). I've been doing this in Calcium and so far it "feels" right.

@tthsqe12
Copy link
Contributor

Ok, but do you have any issues with my prescription for the behavior of the function? There can be controversy here...

@fredrik-johansson
Copy link
Collaborator

No, I agree with everything you said.

@monien
Copy link
Contributor Author

monien commented Apr 24, 2021

Adding roots functionality to fmpz_poly_factor will not work.

One would need to include "fq.h" and "quadic.h" in "fmpz_poly_factor.h" which introduces circular dependencies in the include tree.

@tthsqe12
Copy link
Contributor

I have no good ideas on the interface. Maybe @fredrik-johansson or @wbhart can chime in.

@wbhart
Copy link
Collaborator

wbhart commented May 3, 2021

The way we normally handle the circular reference issue is to put the signatures in both files, but as a comment in the one that can't support the actual signatures. Usually these are placed at the very bottom of the .h file, with a note in the comment explaining where the real signatures are.

This only affects the signature. The implementation remains in the module with the comments rather than the one with the real signatures. See charpoly and minpoly for examples of where this is already done, see for example nmod_mat.h/nmod_poly.h.

@MGessinger
Copy link

MGessinger commented Aug 2, 2021

If I may, I believe the algorithm used to compute padic roots is far from optimal. The roots as computed over the finite field Fp are exactly the "first" (i.e. least significant) digits of the padic expansion. The Euler iteration does not respect that digit, so after just one iteration we lose that piece of info. It also doesn't "refine" roots that agree in low precision, but differ in high precision.

Instead, the following method computes the "digits" one at a time:
Suppose we call the polynomial q, and the prime p (i.e. we want to find roots of q over Qp).

  1. Compute the roots with multiplicity (ri, mi) of q over Z/pZ.
  2. For each pair (ri, mi) define qi(x) = q(p*x+ri)/p^mi.
  3. Return to step 1 with qi instead of q
    This iteration will produce a series of digits {ri}, which correspond exactly to the digits of the root r. Up to a small caveat (that I will come back to in a moment), this algorithm converges exponentially (w.r.t. the padic metric).

To further justify why this method might be of interest, allow me to address the specific properties that @tthsqe12 listed above:

  1. Unfortunately, this method doesn't distinguish roots that only differ after a certain precision. Those roots are reported as duplicate, even though they aren't. Maybe a refinement of this method can allow that?
  2. The two simple roots are reported as [(0,1),(1,1)] in precision 1 and [(5,1),(6,1)] in any higher precision.
  3. This is the caveat mentioned earlier: It doesn't do roots in Qp, it does roots in Zp. I am sure there is some easy rescaling of x that can bring all the roots into Zp (probably something like "divide x by powers of p until the leading coefficient is coprime to p"), and then transform them back after calculating the expansion.
  4. This case would need to be checked explicitly, but that can be done with relative ease: As used in the definition of qi, if ri is a root of qi of multiplicity (up to O(p)), then q(p*x+ri) is divisible by p^mi. Otherwise, this is an "erroneous" root, and can be ignored. Such is the case for q(x) = x^2-5, which reports a double root at ri=0, but q(5x) = 25x^2-5 is not divisible by 25.
  5. The constant polynomial x (and in fact all monomials x^n) return only the digit zero with multiplicity 1 (or n in general), so set however many digits you want.

I am aware that memory managing this will be a nightmare, but there are some optimizations that can be made right off the bat.
If (ri, mi) is the root used to define qi, then the modular projection of qi will be of degree mi, which will in general be less than the degree of q.
Also, if at some point qi == q (defined over Z, not Z/pZ), then the root ri is indeed of multiplicity mi, and its expansion becomes periodic from this point on.

@tthsqe12
Copy link
Contributor

tthsqe12 commented Aug 9, 2021

Indeed, this is a intricate function to write. There are many possible approaches, and I do not expect anything that works to come in at fewer than 1k LOC.

padic_add (y0, y0, tmp, ctx);
}
/* Newton step: x -> x - poly / poly' */
padic_inv (y1, y1, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest a double iteration to compute rt and inv(f'(rt)) at the same time. Otherwise, this inv repeats the previous inve all the time.

Also, in case on many roots, a lifting of the factorisation is much faster than lifting of individual roots.

The precision should be dynamically increased using a lifting chain...

I guess most of this applies to the qadic case as well

Copy link
Contributor

Choose a reason for hiding this comment

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

https://arxiv.org/pdf/1610.06837.pdf
algo 13, steps 1-3

P. Panayi, Computation of Leopoldt’s p-adic regulator, Dissertation, University of East Anglia, 1995.
contains a "certified" p-adic root finder also in the hard cases

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.

6 participants