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.

Sign up for GitHub

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

Jump to bottom

Speedup of the method to reduce ternary quadratic forms in the class TernaryQF #36554

Merged
merged 2 commits into from
Oct 31, 2023
Merged

Speedup of the method to reduce ternary quadratic forms in the class TernaryQF #36554

merged 2 commits into from
Oct 31, 2023

Conversation

tornaria
Copy link
Contributor

@tornaria tornaria commented Oct 26, 2023 •
edited by mkoeppe
Loading

PR from #16965 (rebased from sagemath/sagetrac-mirror@091cbbd)

Resolves #16965

Author: Gustavo Rama

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.

@tornaria
Eliminates the use of matrix constructor in internal loop of the func…
7792961 
…tion sage.quadratic_forms.ternary._reduced_ternary_form_eisenstein_with_matrix.
@tobiasdiez
Copy link
Contributor

Linter is failing

@tornaria
Copy link
Contributor Author

Linter is failing

Thanks for catching it. I took this old commit and rebased, run checks etc. but I didn't think about code style. It should be ok now.

@tornaria
Copy link
Contributor Author

I checked the linter locally with tox -e pycodestyle-minimal which passes everything except some E401 for src/sage/misc/persist.pyx (which is fixed in #36522).

OTOH, tox -e pycodestyle gives tons of failures...

@tobiasdiez
Copy link
Contributor

I checked the linter locally with tox -e pycodestyle-minimal which passes everything except some E401 for src/sage/misc/persist.pyx (which is fixed in #36522).

Thanks! Feel free to set it back to positive review once the linter/tests pass.

OTOH, tox -e pycodestyle gives tons of failures...

Yes, there is still quite a bit work to do 😄

@github-actions
Copy link

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

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 29, 2023
sagemathgh-36554: Speedup of the method to reduce ternary quadratic f…
140d471 
…orms in the class TernaryQF

PR from sagemath#16965 (rebased from https://github.com/sagemath/sagetrac-
mirror/commit/091cbbdf6eed2c8bccca407addd07ff08e429c17)

Resolves sagemath#16965

Author: Gustavo Rama

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.

URL: sagemath#36554
Reported by: Gonzalo Tornaría
Reviewer(s):
@vbraun vbraun merged commit c0221f9 into sagemath:develop Oct 31, 2023
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 31, 2023
@tornaria tornaria deleted the speedup_of_the_method_to_reduce_ternary_quadratic_forms_in_the_class_ternaryqf branch November 2, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c: quadratic forms
Projects
None yet
Milestone
sage-10.2
Development

Successfully merging this pull request may close these issues.

Speedup of the method to reduce ternary quadratic forms in the class TernaryQF
4 participants
@tornaria @tobiasdiez @vbraun @mkoeppe