Skip to content

Add gr_poly_add_scalar, gr_poly_sub_scalar, etc. #2194

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

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

rburing
Copy link
Contributor

@rburing rburing commented Jan 27, 2025

Let me know if there's any way to shorten the code.

@rburing
Copy link
Contributor Author

rburing commented Jan 27, 2025

I'll add gr_poly_addmul_scalar and gr_poly_submul_scalar to this PR tomorrow.

@rburing rburing marked this pull request as draft January 27, 2025 16:41
@rburing rburing force-pushed the gr_poly_add_scalar branch from b431da5 to 5f3a2d2 Compare January 28, 2025 09:20
@rburing rburing marked this pull request as ready for review January 28, 2025 09:27
@rburing rburing force-pushed the gr_poly_add_scalar branch 2 times, most recently from 33590bb to ab2a82d Compare January 29, 2025 13:37
@fredrik-johansson
Copy link
Collaborator

Any idea why the CI is failing?

@rburing rburing force-pushed the gr_poly_add_scalar branch from ab2a82d to d8feeee Compare January 30, 2025 21:28
@rburing
Copy link
Contributor Author

rburing commented Jan 30, 2025

The test

gr_test_iter(R, state, "sub: ui/si/fmpz/fmpq", gr_test_sub_type_variants, iters, test_flags);
is failing for polynomials over mpn_mod. After trying to force some output I get:

sub
Ring of polynomials over Integers mod 9619630419041620901435312400118315361684135181482916166167977550805210517601877109410312712138085742792467808192 (mpn)
which: 4
alias: 0
x = 9079397581980565535
y = 618951719031417411676930303
y (op) y (1) = 9619630419041620901435312400118315361684135181482916166167977550805210517601877109409693760428133722962771443424
x (op) y (2) = 926336713898529537677559173422236530738054845141216017338574778030216341092355*x + 9619630419041620901435312400118315361684135073643129497565418372137328575484942659846867307721977005235714916288*x^2 + 1152921504606846975*x^3

Flint exception (General error):
    
FAIL

Not sure what's going on.

@fredrik-johansson
Copy link
Collaborator

Not sure if this is the only problem, but you need to normalise the output polynomial in the ui, si, fmpz and fmpq methods in the case where len == 0 and c != 0, the reason being that in finite characteristic, c can still convert to zero.

It's unfortunate that the test code doesn't catch this! I think we need some stronger tests for normalisation.

@fredrik-johansson
Copy link
Collaborator

See #2223.

@fredrik-johansson
Copy link
Collaborator

Even so, gr_poly is actually designed to give correct results even when polynomials are not normalised. I wonder why that is not working as intended here.

@rburing rburing force-pushed the gr_poly_add_scalar branch from d8feeee to 53f185d Compare January 31, 2025 09:42
@rburing
Copy link
Contributor Author

rburing commented Jan 31, 2025

@fredrik-johansson Thanks! This should be good to go now (status was used uninitialized in some cases, now fixed).

@fredrik-johansson
Copy link
Collaborator

Great, thank you!

@fredrik-johansson fredrik-johansson merged commit 9f55d76 into flintlib:main Jan 31, 2025
12 checks passed
@rburing rburing deleted the gr_poly_add_scalar branch February 24, 2025 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gr_poly_add_si, gr_poly_sub_si, etc.
2 participants