Skip to content

TropicalGeometry: tropical linear spaces from graphs #4445

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 12 commits into from
Mar 19, 2025

Conversation

YueRen
Copy link
Member

@YueRen YueRen commented Jan 10, 2025

This pull requests adds a new constructor for tropical linear spaces from graphs. Useful (for me) for example for graph rigidity questions (see e.g. here).

@YueRen
Copy link
Member Author

YueRen commented Jan 10, 2025

@ollieclarke8787 Would you be able to review this pull request?

@YueRen YueRen changed the title new: tropical_linear_space for graphs TropicalGeometry: tropical linear spaces from graphs Jan 10, 2025
@YueRen YueRen force-pushed the yr/TropicalLinearSpace branch from 274f8c2 to b86b6a1 Compare January 10, 2025 14:13
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.40%. Comparing base (3e86180) to head (67a77f1).
Report is 161 commits behind head on master.

Files with missing lines Patch % Lines
src/TropicalGeometry/linear_space.jl 81.81% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4445      +/-   ##
==========================================
- Coverage   84.55%   84.40%   -0.16%     
==========================================
  Files         672      673       +1     
  Lines       88880    90278    +1398     
==========================================
+ Hits        75152    76198    +1046     
- Misses      13728    14080     +352     
Files with missing lines Coverage Δ
src/TropicalGeometry/linear_space.jl 85.71% <81.81%> (+4.59%) ⬆️

... and 187 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@ollieclarke8787 ollieclarke8787 left a comment

Choose a reason for hiding this comment

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

Code correctly produces Bergman fan of the cycle matroid the graph. The example of K_3 is also correct.

@YueRen YueRen force-pushed the yr/TropicalLinearSpace branch 3 times, most recently from bcd1d94 to 69551de Compare January 17, 2025 15:03
@YueRen YueRen force-pushed the yr/TropicalLinearSpace branch from 69551de to 9079fd8 Compare January 24, 2025 08:52
@YueRen

This comment was marked as off-topic.

@fingolfin

This comment was marked as off-topic.

@thofma

This comment was marked as off-topic.

@fingolfin
Copy link
Member

I am not sure who from the inside is able to review (and approve!) this tropical geometry PR. Perhaps someone from Berlin, e.g. @lkastner @benlorenz @micjoswig ?

@lkastner lkastner requested a review from micjoswig February 25, 2025 16:08
Copy link
Member

@micjoswig micjoswig Feb 26, 2025

Choose a reason for hiding this comment

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

The docs on tropical linear spaces (and the rest of tropical geometry) need to state explicitly where the various objects (tropical varieties, tropical linear spaces etc) actually live. Candidates include R^n or the tropical projective torus R^n/R1 (in the homogeneous setting). Either there are choices for the user to make, or there are fixed global decisions.

Specifically, tropical linear spaces may also be viewed as tropical polytopes. And then there are more choices, for they can be considered in a tropical projective torus or in a tropical projective space. I guess, none of this is currently supported in OSCAR.

The role of Bergman fans and constant vs non-constant coefficients must be explained.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good points, I will adjust the documentation accordingly.

@micjoswig micjoswig self-requested a review February 26, 2025 09:55
@fingolfin fingolfin removed the triage label Feb 26, 2025
This is technically not necessary, as passing `nothing` to
tropical_linear_space inside the function is valid, but this makes it
more consistent and maybe easier to read
Added a note that tropical linear spaces in OSCAR (currently) live in
euclidean space and not in the tropical torus
Constructing tropical linear spaces over the rationals go through an
optimised polymake function and thus they lack certain
attributes (because computing them would be extra work)
@YueRen
Copy link
Member Author

YueRen commented Mar 14, 2025

Besides enabling the constructing of tropical linear spaces from graphs, the following have changed:

  1. Replaced how optional TropicalSemiringMap are written. Before the function signature included nu::Union{Nothing,TropicalSemiringMap}=nothing and the function body included code that set nu when nu==nothing. Now this is done in the function signature directly like:
function tropical_linear_space(I::MPolyIdeal, nu::TropicalSemiringMap=tropical_semiring_map(coefficient_ring(I)); weighted_polyhedral_complex_only::Bool=false)
  1. Adding a few sentences to the documentation, that emphasize that
  • tropical linear spaces in OSCAR live in the euclidean space, not the tropical torus
  • tropical linear spaces under a trivial valuation are Bergman fans and they may have different structures depending on how they are constructed

Copy link
Member

@micjoswig micjoswig left a comment

Choose a reason for hiding this comment

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

I think it is OK now.

Some minor edits, e.g., improving the docs further, can be made after the merge. I find that easier than messing with the code in an external repo.

@micjoswig micjoswig merged commit 14ed258 into oscar-system:master Mar 19, 2025
35 checks passed
@benlorenz benlorenz added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label Mar 21, 2025
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: tropical geometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants