Add generic Ore polynomial module by rburing · Pull Request #2299 · flintlib/flint

rburing · 2025-04-12T11:01:30Z

This contains only the basic structure so far, such as memory management, additive arithmetic, and multiplication from the left by an element of the base ring.

Part of Wishlist for D-finite/holonomic functions #1881

Feedback welcome!

We would like to use this data type as input to differential equation solvers, so we don't need the multiplicative structure initially, though it would be nice to have it in the future.

Multiplicative operations (to be defined later) depend on the Ore context object, so I figured that all operations should take the Ore context as input, to make the interface uniform. So now the interface looks like gr_mpoly, and the basic operations implemented so far call gr_poly methods.

~~Since the base ring will typically be polynomial, should gr_ore_poly_scalar_mul and GR_ORE_POLY_ELEM_CTX be named differently? Maybe using base / BASE?~~

~~I also tried to make the module "optional" in the sense that gr_mpoly is, let me know if I missed something in this regard.~~

cc @mezzarobba

src/ore_poly_types.h

mezzarobba · 2025-04-12T13:29:21Z

On Saturday, 12 April 2025 Ricardo Buring wrote: > +typedef enum { + ORE_OPERATOR_DERIVATIVE, + ORE_OPERATOR_EULER_DERIVATIVE, + /* TODO: Shift operator, etc. */ +} +ore_operator_t; I named this enum (and the value prefix) improperly. What should the name be?

Maybe gr_ore_poly_which_operator, for consistency with gr_which_structure?

…

-- Marc

rburing · 2025-04-12T18:40:13Z

Some/most of the arithmetic/setter methods (gr_ore_poly_add for example) are not getting called in the ring tests.

I'm not sure what I did wrong since I did put them in the method table.

Edit: Never mind, I put some NULL pointers in there which is not allowed except to indicate the end of the table.

src/gr.h

fredrik-johansson · 2025-04-15T11:40:06Z

So, working with Ore polynomials over say R[x], one might indeed want to do mixed operations with operands from either R or R[x]. The term "scalar" being ambiguous, instead of adding many extra methods, perhaps just define gr_ore_poly_add_other etc. that handles all cases?

fredrik-johansson · 2025-04-15T11:41:14Z

I don't see the need for ore_poly.h and ore_poly_types.h.

fredrik-johansson · 2025-04-15T11:41:46Z

Most of the one-line trivial methods could go in a single file.

fredrik-johansson · 2025-04-15T11:46:41Z

Multiplicative operations (to be defined later) depend on the Ore context object, so I figured that all operations should take the Ore context as input, to make the interface uniform.

Yes, this is the right way to do it. The way gr_poly and gr_mat work directly with the element context is an exception rather than the rule, and at some point we should add a public interface for those types using a poly/mat context as well.

rburing · 2025-04-18T12:30:49Z

I reorganized the code to collect the wrapper functions in one file, and applied all the other suggestions above. I removed all the methods with scalar in the name and added some other methods, indeed this looks much better.

Let me know if this needs any further changes.

fredrik-johansson · 2025-04-18T12:33:30Z

Looks good. The only thing missing is documentation.

doc/source/gr_ore_poly.rst

fredrik-johansson · 2025-04-18T14:29:25Z

Something that doesn't seem to be well specified in the documentation is the generator name (set_gen_name / string conversion methods). If I'm in R[x][D], say, shouldn't x already be named the base ring while the Ore ring generator is D?

fredrik-johansson · 2025-04-18T16:58:40Z

In fact, shouldn't the generator x (given as an element of the base ring) be a parameter to the Ore polynomial ring constructor? In general, the base ring could be something multivariate.

rburing · 2025-04-29T12:30:09Z

I updated the context and constructor to include an index of a generator of the base ring, and (hopefully) clarified the documentation. I think the restriction to generators of the base ring (rather than elements of the base ring) is reasonable, and e.g. in the univariate case it saves the user from having to define a variable that will probably never be used (e.g. multiplication by the variable will be replaced by a shift). Let me know if this is alright.

mezzarobba · 2025-04-30T08:27:14Z

On Tuesday, 29 April 2025 Ricardo Buring wrote: @rburing requested your review on: flintlib/flint#2299 Add generic Ore polynomial module.

Sure, I can take a look, but maybe not before next week. Thanks for your work on this!

rburing · 2025-05-05T09:26:24Z

Another question: How should the q parameter for q-shifts or q-derivations be handled? The current gr_ctx_init_gr_ore_poly and the struct _gr_ore_poly_ctx_struct do not refer to it. Can this be left for later? I guess the main question is: do we want to put an extra parameter (again an index into the list of generators or a gr_srcptr?) already in gr_ctx_init_gr_ore_poly, or add a separate second constructor later (or break source compatibility by adding an extra parameter to gr_ctx_init_gr_ore_poly later)?

rburing · 2025-05-15T16:35:39Z

Thanks for the extensive feedback @mezzarobba, I think it looks better now.

What do you think about using the name ore_algebra_t for the enum? With values ORE_ALGEBRA_STANDARD_DERIVATIVE and ORE_ALGEBRA_EULER_DERIVATIVE for now.

@fredrik-johansson Should I keep entry in the name of gr_ore_poly_entry_ptr for consistency with gr_poly, or use coeff instead?

fredrik-johansson · 2025-05-16T09:15:05Z

I think the gr_poly method is misnamed and should use coeff as well.

mezzarobba · 2025-05-16T14:39:13Z

What do you think about using the name ore_algebra_t for the enum? With values ORE_ALGEBRA_STANDARD_DERIVATIVE and ORE_ALGEBRA_EULER_DERIVATIVE for now.

Sounds good too. I think I would call d/dx DERIVATIVE instead of STANDARD_DERIVATIVE, and you could add SHIFT (or maybe FORWARD_SHIFT) and FORWARD_DIFFERENCE right away.

mezzarobba · 2025-09-17T19:24:00Z

I added support for general (σ,δ) and implemented a number of standard (σ,δ) pairs, see https://github.com/mezzarobba/flint/tree/gr_ore_poly. Any feedback welcome. (Ricardo, feel free to merge my branch into this PR if you like.)

mezzarobba · 2025-09-17T20:01:09Z

I should add that most predefined operator types are currently limited to base rings of type gr_poly but this restriction should be easy to lift if a few methods like gr_derivative or gr_subs are added to the general interface of generic rings. (I didn't want to embark in that now.)

This contains only the basic structure so far, such as memory management, additive arithmetic, and multiplication from the left by an element of the base ring.

Mark some functions as static, add ngens to pass ring test, fix mismatch of sigma_delta_compose function name in header.

rburing · 2026-03-22T13:28:14Z

I merged the implementation of Ore polynomial multiplication by @vioneers into this branch, and rebased it to resolve merge conflicts and a few technical issues (due to other developments in the codebase in the meantime).

rburing · 2026-03-22T14:13:31Z

I don't really understand why the tests (flint_fprintf with ASAN and codecov make check) are failing.

rburing · 2026-03-24T13:40:31Z

For the flint_fprintf test: it seems this

flint/src/generic_files/io.c

Line 206 in baf1c95

#define IS_FLINT_TYPE(ip, str) (memcmp(ip, str "}", sizeof(str)) == 0)

can read past the end of the buffer, e.g. when there is a {%gr_ formatter near the end of the format string and str is too large (here "gr_ore_poly" happens to be large).

How should we deal with this (pre-existing) issue?

fredrik-johansson · 2026-03-24T15:02:09Z

Good catch. I guess memcmp should be something like strcmp or strncmp.

mezzarobba · 2026-03-25T10:15:37Z

src/gr_ore_poly/test/t-mul.c

+                found_algebra = 1;
+            else
+            {
+                if (algebra != ORE_ALGEBRA_FROBENIUS && ctx->which_ring == GR_CTX_GR_POLY)


why don't you like the Frobenius case? and what is this doing with the base ring??

mezzarobba · 2026-03-25T10:18:04Z

src/gr_ore_poly/test/t-mul.c

+        while(!found_algebra)
+        {
+            // Random context
+            gr_ore_poly_ctx_init_randtest2(ctx, ore_ctx, state);


Only the ctx and ore_ctx from the last iteration are cleared.

mezzarobba · 2026-03-25T10:18:29Z

src/gr_ore_poly/test/t-mul.c

+        gr_ore_poly_ctx_t ore_ctx;
+        slong reps;
+        ore_algebra_t algebra;
+        int found_algebra = 0;


I don't see the need for this flag.

mezzarobba · 2026-03-25T10:33:19Z

The powering tests from the generic test suite are slow in the Mahler case (reached with FLINT_TEST_MULTIPLIER=10); we need a way to disable them or limit them to really really tiny examples. Maybe something like this:

diff --git a/src/gr_ore_poly/mul.c b/src/gr_ore_poly/mul.c
index 377ed6d78..d2d9e1999 100644
--- a/src/gr_ore_poly/mul.c
+++ b/src/gr_ore_poly/mul.c
@@ -146,6 +146,9 @@ int gr_ore_poly_mul(gr_ore_poly_t res, const gr_ore_poly_t poly1, const gr_ore_p
     if (len1 == 0 || len2 == 0)
         return gr_ore_poly_zero(res, ctx);

+    if (len1 + len2 - 1 > ctx->size_limit)
+        return GR_UNABLE;
+
     if (GR_ORE_POLY_CTX(ctx)->sigma_delta == NULL)
         return GR_UNABLE;

diff --git a/src/gr_ore_poly/test/t-ring.c b/src/gr_ore_poly/test/t-ring.c
index 82768e04f..e0e4f860b 100644
--- a/src/gr_ore_poly/test/t-ring.c
+++ b/src/gr_ore_poly/test/t-ring.c
@@ -28,6 +28,9 @@ TEST_FUNCTION_START(gr_ore_poly_ring, state)
         slong reps;

         gr_ore_poly_ctx_init_randtest2(ctx, ore_ctx, state);
+        if (GR_ORE_POLY_CTX(ore_ctx)->which_algebra == ORE_ALGEBRA_MAHLER
+            || GR_ORE_POLY_CTX(ore_ctx)->which_algebra == ORE_ALGEBRA_Q_SHIFT)
+            ore_ctx->size_limit = 2;

         if (gr_ctx_is_finite(ctx) == T_TRUE ||
             gr_ctx_has_real_prec(ctx) == T_TRUE)

mezzarobba · 2026-03-25T11:07:00Z

src/gr_ore_poly/test/t-mul.c

+#include "gr_vec.h"
+
+// same as GR_MUST_SUCCEED but details of the failing printed
+#define MUST_OK(expr) do { \


I thought this was temporary debugging stuff that would be removed in the final code?

Note that requiring all products to succeed is not correct since it may happen, for instance, that multiplications in the base ring return GR_ENABLE.

But, btw: is this test file needed at all in the end? Or is it subsumed by t-ring?

mezzarobba · 2026-03-25T11:07:46Z

src/gr_ore_poly/test/t-mul.c

+            reps = 3;
+        }
+
+        /* Hack: for string conversion tests, make sure we don't have


What string conversion tests? (Eager copy-paste useless here, or am I missing something?)

mezzarobba · 2026-03-25T11:10:19Z

src/gr_ore_poly/mul.c

@@ -0,0 +1,170 @@
+/*
+    Copyright (C) 2026 Maria Neagoie, supervised by Marc Mezzarobba and Ricardo Buring


I don't think the “supervised by…” part is necessary :-)

mezzarobba · 2026-03-25T12:15:26Z

src/gr_ore_poly/mul.c

+    slong el_size = gr_ctx_sizeof_elem(GR_ORE_POLY_ELEM_CTX(ctx));
+    int status = GR_SUCCESS;
+
+    if (GR_ORE_POLY_CTX(ctx)->sigma_delta == NULL)


when/why would that be null?

mezzarobba · 2026-03-25T12:20:24Z

src/gr_ore_poly/mul.c

+    TMP_INIT;
+    TMP_START;
+
+    gr_ptr temps = TMP_ALLOC(el_size);


why not GR_TMP_INIT?

mezzarobba · 2026-03-25T12:24:18Z

src/gr_ore_poly/mul.c

+    }
+    else
+    {
+        // Output initialized at 0


why do you need to zero all coefficients in the non-aliased case?

mezzarobba · 2026-03-25T12:57:42Z

doc/source/gr_ore_poly.rst

+    Sets *res* to *poly* multiplied by *c* (or *x* multiplied by *poly*)
+    which must be an element of or coercible to the coefficient ring.
+
+.. function:: int _gr_ore_poly_lmul_gen(gr_ptr res, gr_srcptr poly, slong len, gr_ore_poly_ctx_t ctx)


If you list the underscore variants, it would be good to document the assumptions they make.

mezzarobba · 2026-03-25T13:02:33Z

src/gr_ore_poly/mul.c

+    // Initialize Q = b, then Q = D^i * b.
+    gr_ore_poly_t Q;
+    gr_ore_poly_init2(Q, len2, ctx);
+    for (slong i = 0; i < len2; i++)


doesn't gr_set work on Ore polynomials?

mezzarobba · 2026-03-25T13:04:39Z

src/gr_ore_poly/mul.c

+        {
+            gr_srcptr qj = GR_ENTRY(Q->coeffs, j, el_size);
+
+            // Compute a_i * q_j


gr_addmul?

It's probably better to do it with gr_add/gr_mul for now. Many rings don't implement an optimized gr_addmul and the generic version will do a temporary allocation each time.

ok, thanks!

fredrik-johansson · 2026-03-25T19:34:17Z

src/gr_ore_poly.h

+
+void gr_ore_poly_ctx_init(gr_ore_poly_ctx_t ctx, gr_ctx_t base_ring, slong base_var, const ore_algebra_t which_algebra);
+int gr_ore_poly_ctx_init_q_shift(gr_ore_poly_ctx_t ctx, gr_ctx_t base_ring, slong base_var, gr_srcptr q);
+int gr_ore_poly_ctx_init_mahler(gr_ore_poly_ctx_t ctx, gr_ctx_t base_ring, slong base_var, long mahler_base);


The long type should be avoided; change to slong?

rburing force-pushed the gr_ore_poly branch from baa68d2 to f789ae5 Compare April 12, 2025 11:17

rburing added the generics label Apr 12, 2025

rburing commented Apr 12, 2025

View reviewed changes

src/ore_poly_types.h Outdated Show resolved Hide resolved

rburing force-pushed the gr_ore_poly branch from f789ae5 to c21f074 Compare April 12, 2025 17:26

rburing force-pushed the gr_ore_poly branch 2 times, most recently from 4cfe741 to c5d790c Compare April 13, 2025 13:07

fredrik-johansson reviewed Apr 15, 2025

View reviewed changes

src/gr.h Outdated Show resolved Hide resolved

fredrik-johansson reviewed Apr 15, 2025

View reviewed changes

src/gr.h Outdated Show resolved Hide resolved

rburing force-pushed the gr_ore_poly branch from c5d790c to 2db94fe Compare April 18, 2025 12:23

rburing force-pushed the gr_ore_poly branch 2 times, most recently from e5ac464 to 6323582 Compare April 18, 2025 13:37

fredrik-johansson reviewed Apr 18, 2025

View reviewed changes

doc/source/gr_ore_poly.rst Outdated Show resolved Hide resolved

rburing force-pushed the gr_ore_poly branch from 6323582 to bdc7052 Compare April 29, 2025 12:18

rburing requested a review from mezzarobba April 29, 2025 14:21

rburing force-pushed the gr_ore_poly branch 2 times, most recently from 5b16adc to ee4d57a Compare April 29, 2025 14:49

rburing mentioned this pull request May 5, 2025

Wishlist for D-finite/holonomic functions #1881

Open

12 tasks

rburing force-pushed the gr_ore_poly branch 2 times, most recently from f4704c9 to d2f0653 Compare May 21, 2025 09:38

rburing mentioned this pull request May 22, 2025

Add gr_poly_coeff_ptr and deprecate gr_poly_entry_ptr #2321

Merged

rburing force-pushed the gr_ore_poly branch from d2f0653 to 7997e6b Compare July 29, 2025 09:52

rburing and others added 5 commits March 22, 2026 13:22

Add generic Ore polynomial module

342315f

This contains only the basic structure so far, such as memory management, additive arithmetic, and multiplication from the left by an element of the base ring.

add gr_ctx_init_random_{poly,mpoly,series}

b0712d0

gr_ore_poly: sigma, delta

3f5090e

added gr_ore_poly multiplication, testing and documentation

6ee55f1

gr_ore_poly: fixes after rebase

9cf621c

Mark some functions as static, add ngens to pass ring test, fix mismatch of sigma_delta_compose function name in header.

rburing force-pushed the gr_ore_poly branch from 83b0a53 to 9cf621c Compare March 22, 2026 13:27

Fix read past end of buffer in flint_fprintf

f325267

mezzarobba reviewed Mar 25, 2026

View reviewed changes

src/gr_ore_poly/mul.c

TMP_INIT;

TMP_START;

gr_ptr temps = TMP_ALLOC(el_size);

Copy link
Copy Markdown

Contributor

mezzarobba Mar 25, 2026

Choose a reason for hiding this comment

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

why not GR_TMP_INIT?

mezzarobba reviewed Mar 25, 2026

View reviewed changes

fredrik-johansson reviewed Mar 25, 2026

View reviewed changes

		@@ -0,0 +1,170 @@
		/*
		Copyright (C) 2026 Maria Neagoie, supervised by Marc Mezzarobba and Ricardo Buring

Conversation

rburing commented Apr 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mezzarobba commented Apr 12, 2025 via email

Uh oh!

rburing commented Apr 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fredrik-johansson commented Apr 15, 2025

Uh oh!

fredrik-johansson commented Apr 15, 2025

Uh oh!

fredrik-johansson commented Apr 15, 2025

Uh oh!

fredrik-johansson commented Apr 15, 2025

Uh oh!

rburing commented Apr 18, 2025

Uh oh!

fredrik-johansson commented Apr 18, 2025

Uh oh!

Uh oh!

fredrik-johansson commented Apr 18, 2025

Uh oh!

fredrik-johansson commented Apr 18, 2025

Uh oh!

rburing commented Apr 29, 2025

Uh oh!

mezzarobba commented Apr 30, 2025 via email

Uh oh!

rburing commented May 5, 2025

Uh oh!

rburing commented May 15, 2025

Uh oh!

fredrik-johansson commented May 16, 2025

Uh oh!

mezzarobba commented May 16, 2025

Uh oh!

mezzarobba commented Sep 17, 2025

Uh oh!

mezzarobba commented Sep 17, 2025

Uh oh!

rburing commented Mar 22, 2026

Uh oh!

rburing commented Mar 22, 2026

Uh oh!

rburing commented Mar 24, 2026

Uh oh!

fredrik-johansson commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mezzarobba Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mezzarobba commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

rburing commented Apr 12, 2025 •

edited

Loading

rburing commented Apr 12, 2025 •

edited

Loading

fredrik-johansson commented Mar 24, 2026 •

edited

Loading

mezzarobba Mar 25, 2026 •

edited

Loading

mezzarobba commented Mar 25, 2026 •

edited

Loading