Skip to content

Some fixes & improvements to autogenerated flint .pxd#37193

Merged
vbraun merged 5 commits intosagemath:developfrom
mezzarobba:flint-autogen-tweaks
Feb 13, 2024
Merged

Some fixes & improvements to autogenerated flint .pxd#37193
vbraun merged 5 commits intosagemath:developfrom
mezzarobba:flint-autogen-tweaks

Conversation

@mezzarobba
Copy link
Copy Markdown
Member

@mezzarobba mezzarobba commented Jan 30, 2024

  • flint autogen: fields of a[rc]b_poly_struct
  • flint autogen: remove two typedefs that don't exist in flint
  • move fix from Fix build when linbox is build with fplll support #37064 to the flint interface generator
  • flint autogen: add warnings about generated files to the templates
  • regenerate flint wrappers

@mezzarobba mezzarobba requested a review from videlec January 30, 2024 13:47
@videlec
Copy link
Copy Markdown
Contributor

videlec commented Jan 30, 2024

Interesting

sage -t --long --random-seed=286735480429121101562228604801325644303 sage/rings/tests.py
**********************************************************************
File "sage/rings/tests.py", line 453, in sage.rings.tests.?
Failed example:
    test_karatsuba_multiplication(QQbar, 3, 3, numtests=2)    # long time, needs sage.rings.number_field
Expected nothing
Got:
    Random testing has revealed a problem in test_karatsuba_multiplication
    Please report this bug!  You may be the first
    person in the world to have seen this problem.
    Please include this random seed in your bug report:
    Random seed: 327619870453015189736949675324415167773
    ValueError('Multiplication failed')
**********************************************************************
1 item had failures:
   1 of  13 in sage.rings.tests.?
    [62 tests, 1 failure, 1191.47 s]

@tornaria
Copy link
Copy Markdown
Member

tornaria commented Jan 30, 2024

The autogen scripts seem to be run manually, and the output files are committed to source. Thus, it doesn't make sense to ship these scripts with sage_setup, does it?

@mezzarobba
Copy link
Copy Markdown
Member Author

mezzarobba commented Jan 31, 2024

Interesting

sage -t --long --random-seed=286735480429121101562228604801325644303 sage/rings/tests.py
**********************************************************************
File "sage/rings/tests.py", line 453, in sage.rings.tests.?
Failed example:
    test_karatsuba_multiplication(QQbar, 3, 3, numtests=2)    # long time, needs sage.rings.number_field
Expected nothing
Got:
    Random testing has revealed a problem in test_karatsuba_multiplication
    Please report this bug!  You may be the first
    person in the world to have seen this problem.
    Please include this random seed in your bug report:
    Random seed: 327619870453015189736949675324415167773
    ValueError('Multiplication failed')
**********************************************************************
1 item had failures:
   1 of  13 in sage.rings.tests.?
    [62 tests, 1 failure, 1191.47 s]

Can't reproduce locally.

@mezzarobba
Copy link
Copy Markdown
Member Author

mezzarobba commented Jan 31, 2024

The autogen scripts seem to be run manually, and the output files are committed to source.

Yes.

Thus, it doesn't make sense to ship these scripts with sage_setup, does it?

I think I agree.

@mezzarobba
flint autogen: fields of a[rc]b_poly_struct
d823c43 
Rationale: accessing the coefficients of degree bewteen length and alloc
of a polynomial is done via pol->coeffs, there is (afaik) no macro for
that. And these fields are documented.
@mezzarobba
flint autogen: remove two typedefs that don't exist in flint
956ab2c
@mezzarobba
move fix from sagemath#37064 to the flint interface generator
98c3871
@mezzarobba
flint autogen: add warnings about generated files to the templates
c0ba507 
(it didn't seem worth the trouble modifying the generator itself so that
it inserts the warning in the correct place...)
@mezzarobba
regenerate flint wrappers
c179886 
using flint commit 3e2c3a3e091106a25ca9c6fba28e02f2cbcd654a, which is a
bit more recent than the flint version sage officially depends on but
matches the API currently exposed in the generated files
@mezzarobba mezzarobba force-pushed the flint-autogen-tweaks branch from bd29c6c to c179886 Compare February 3, 2024 12:53
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 3, 2024

Documentation preview for this PR (built with commit c179886; changes) is ready! 🎉

@mezzarobba mezzarobba mentioned this pull request Feb 4, 2024
Closed
@mezzarobba
Copy link
Copy Markdown
Member Author

mezzarobba commented Feb 4, 2024

The two test failures are now #37216 and #37238 and don't seem to have anything to do with this PR. @videlec, thank you for reviewing the changes; can I interpret the gh approval as positive review?

vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 7, 2024
a02ae74 
sagemathgh-37193: Some fixes & improvements to autogenerated flint .pxd
    
- flint autogen: fields of a[rc]b_poly_struct
- flint autogen: remove two typedefs that don't exist in flint
- move fix from sagemath#37064 to the flint interface generator
- flint autogen: add warnings about generated files to the templates
- regenerate flint wrappers
    
URL: sagemath#37193
Reported by: Marc Mezzarobba
Reviewer(s): Vincent Delecroix
@vbraun vbraun merged commit adee77d into sagemath:develop Feb 13, 2024
@mezzarobba mezzarobba mentioned this pull request Mar 7, 2024
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@videlec videlec videlec approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

sage-10.3

Development

Successfully merging this pull request may close these issues.

5 participants

@mezzarobba @videlec @tornaria @vbraun @mkoeppe