Skip to content

New wrapper for Singular triangular decompositions#4765

Merged
afkafkafk13 merged 12 commits intooscar-system:masterfrom
YueRen:yr/triangularDecomposition
Apr 2, 2025
Merged

New wrapper for Singular triangular decompositions#4765
afkafkafk13 merged 12 commits intooscar-system:masterfrom
YueRen:yr/triangularDecomposition

Conversation

@YueRen
Copy link
Copy Markdown
Member

@YueRen YueRen commented Mar 28, 2025

And sorry for the unnecessary deletion of a trailing whitespace in docs/src/CommutativeAlgebra/ideals.md, my editor is overzealous at times.

@YueRen YueRen added topic: commutative algebra release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Mar 28, 2025
@YueRen YueRen requested a review from jankoboehm March 28, 2025 10:47
@thofma
Copy link
Copy Markdown
Collaborator

thofma commented Mar 28, 2025

Minor nitpick: I think we don't capitalize anything, not even names. So the algorithm symbols should also all be lowercase (and snakecase). (For example, the option for groebner bases is called algorithm = :buchberger).

So I guess it would be :lazard, :lazard_factorized etc

@YueRen YueRen force-pushed the yr/triangularDecomposition branch from 909e43a to 4616668 Compare March 28, 2025 11:00
@YueRen
Copy link
Copy Markdown
Member Author

YueRen commented Mar 28, 2025

Minor nitpick: I think we don't capitalize anything, not even names. So the algorithm symbols should also all be lowercase (and snakecase). (For example, the option for groebner bases is called algorithm = :buchberger).

So I guess it would be :lazard, :lazard_factorized etc

No problem, that was an easy change.

Comment thread docs/oscar_references.bib Outdated
Comment thread docs/oscar_references.bib Outdated
Comment thread src/Rings/mpoly-ideals.jl Outdated
Comment thread src/Rings/mpoly-ideals.jl Outdated
YueRen and others added 4 commits March 28, 2025 12:13
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Comment thread src/Rings/mpoly-ideals.jl Outdated
@YueRen YueRen force-pushed the yr/triangularDecomposition branch 2 times, most recently from 302f6f1 to ba664c3 Compare March 28, 2025 14:22
@YueRen YueRen force-pushed the yr/triangularDecomposition branch from ba664c3 to 967d5ad Compare March 28, 2025 14:22
Copy link
Copy Markdown
Collaborator

@afkafkafk13 afkafkafk13 left a comment

Choose a reason for hiding this comment

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

Looks fine. Thank you for doing this.

Just as a sideremark: the doctests do not completely test the example, as some of the results are not spelled out due to size. But I do not see this as a problem for this particular code, as it is only wrapping a (stable) Singular-library command.

@YueRen
Copy link
Copy Markdown
Member Author

YueRen commented Apr 1, 2025

@afkafkafk13 I have added the option to specify the ordering of the variables for the triangular decomposition (which I need for my tropical code) through a keyword argument ord (similar as in groebner_basis). The triangular set starts with a univariate polynomial in ord[1], a bivariate polynomial in ord[1] and ord[2], etc. The default is the same as in Singular (ord[1] is the last variable, ord[2] is the second-last variable, etc).

Comment thread src/Rings/mpoly-ideals.jl Outdated
YueRen and others added 2 commits April 1, 2025 14:05
Co-authored-by: Anne Frühbis-Krüger <77079696+afkafkafk13@users.noreply.github.com>
@afkafkafk13
Copy link
Copy Markdown
Collaborator

@YueRen : Resolving the conflict in the bib-file seems to have messed up the file, but I do not see what goes wrong. (Everything was fine with your addition, master was fine with the addition of Lan71, but now bibtool complains)

@YueRen
Copy link
Copy Markdown
Member Author

YueRen commented Apr 2, 2025

The bibliography should be fixed now, just a problem with the ordering of the bib entries.

@afkafkafk13 afkafkafk13 enabled auto-merge (squash) April 2, 2025 14:34
@afkafkafk13 afkafkafk13 merged commit ba4bbe4 into oscar-system:master Apr 2, 2025
32 checks passed
fieker pushed a commit that referenced this pull request May 16, 2025
* CommutativeAlgebra: new triangular decomposition wrapper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: commutative algebra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants