Skip to content

fix exponentation of tropical polynomials #4767

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

Conversation

YueRen
Copy link
Member

@YueRen YueRen commented Mar 31, 2025

Solves a bug reported by @shelbycox via slack.

@thofma
Copy link
Collaborator

thofma commented Mar 31, 2025

Do you have a reference for the characteristic of semirings? (Other than a stackexchange link)

@YueRen
Copy link
Member Author

YueRen commented Mar 31, 2025

Do you have a reference for the characteristic of semirings? (Other than a stackexchange link)

Not at hand, but I'll ask around.

@YueRen
Copy link
Member Author

YueRen commented Mar 31, 2025

It seems that I was mistaken. The correct characteristic of the tropical numbers (or any idempotent semiring) is 1. Maybe it's better to remove that function then, in case some other Oscar function implicitly assumes that characteristic is either 0 or prime.

@YueRen YueRen force-pushed the yr/tropicalSemiringFunctionalities branch from 02d1371 to f4298e2 Compare March 31, 2025 09:18
@YueRen YueRen force-pushed the yr/tropicalSemiringFunctionalities branch from 225c78e to 40f6146 Compare March 31, 2025 09:25
@YueRen YueRen changed the title Yr/tropical semiring functionalities fix exponentation of tropical polynomials Mar 31, 2025
@YueRen YueRen added topic: tropical geometry release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Mar 31, 2025
@afkafkafk13
Copy link
Collaborator

It seems that I was mistaken. The correct characteristic of the tropical numbers (or any idempotent semiring) is 1. Maybe it's better to remove that function then, in case some other Oscar function implicitly assumes that characteristic is either 0 or prime.

It might also be useful to place a meaningful error pointing out that the characteristic of the tropical semiring is not available by choice and not by chance (as it is always 1 and might confuse ring functionality).

Copy link
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 now.

@YueRen
Copy link
Member Author

YueRen commented Apr 1, 2025

It seems that I was mistaken. The correct characteristic of the tropical numbers (or any idempotent semiring) is 1. Maybe it's better to remove that function then, in case some other Oscar function implicitly assumes that characteristic is either 0 or prime.

It might also be useful to place a meaningful error pointing out that the characteristic of the tropical semiring is not available by choice and not by chance (as it is always 1 and might confuse ring functionality).

I have added an error that the characteristic of tropical semirings is not supported. I've thought of ways to make the error more verbose, but what elaboration makes sense strongly depends on how the user encounters the error.

@afkafkafk13
Copy link
Collaborator

It seems that I was mistaken. The correct characteristic of the tropical numbers (or any idempotent semiring) is 1. Maybe it's better to remove that function then, in case some other Oscar function implicitly assumes that characteristic is either 0 or prime.

It might also be useful to place a meaningful error pointing out that the characteristic of the tropical semiring is not available by choice and not by chance (as it is always 1 and might confuse ring functionality).

I have added an error that the characteristic of tropical semirings is not supported. I've thought of ways to make the error more verbose, but what elaboration makes sense strongly depends on how the user encounters the error.

I alreay saw it and approved it. The degree of elaboration in the error message is fine.

@afkafkafk13 afkafkafk13 merged commit 2d78be0 into oscar-system:master Apr 2, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: tropical geometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants