Skip to content

Addition of new presentations to Chow rings of matroids #39359

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 68 commits into from
Jul 25, 2025

Conversation

25shriya
Copy link
Contributor

@25shriya 25shriya commented Jan 21, 2025

This PR focuses on adding the atom-free and simplicial presentation of Chow rings of matroids. see relevant issue
The former is taken from Definition 4.1 of this paper and the latter from Definition 3.2.1 of this paper.

@tscrim

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@25shriya 25shriya changed the title Added new presentations to Chow rings of matroids Addition of new presentations to Chow rings of matroids Jan 21, 2025
Copy link

github-actions bot commented Jan 21, 2025

Documentation preview for this PR (built with commit 5f5d545; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Almost ready; just a few more small things.

@25shriya
Copy link
Contributor Author

@tscrim Do you just mean adding the two lines from sage.rings.rational_field import QQ and q = QQ['q'].gen()? I'm not sure I understand.

@tscrim
Copy link
Collaborator

tscrim commented Jun 14, 2025

I had 2 things in my previous comment (which you can see in the filed changed), and I was referring to this part:

It might also be good to specify the parent for vector() below is q.parent().

@25shriya
Copy link
Contributor Author

Right, understood. Do you mean a check for gchi.parent() == q.parent()?

@tscrim
Copy link
Collaborator

tscrim commented Jun 14, 2025

I meant what I said, add it as an argument to vector to ensure the parent is correct.

@tscrim
Copy link
Collaborator

tscrim commented Jun 16, 2025

@25shriya Some lint issues with trailing whitespace.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Thank you. One last little thing before a positive review, and please check that it does work.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Thank you. Let's get this in.

@25shriya
Copy link
Contributor Author

25shriya commented Jul 15, 2025

Why has the label changed? Shouldn't this be merged by now?

@tscrim
Copy link
Collaborator

tscrim commented Jul 15, 2025

The "smarter"-than-us-auto-bot decided to later revert it back because of the merge. Thanks for catching this.

@25shriya
Copy link
Contributor Author

Should I ignore the build-doc failure? Looks like an error that's not a part of the code I wrote.

@tscrim
Copy link
Collaborator

tscrim commented Jul 16, 2025

It didn't run properly, so I am re-running it.

vbraun pushed a commit to vbraun/sage that referenced this pull request Jul 18, 2025
sagemathgh-39359: Addition of new presentations to Chow rings of matroids
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

This PR focuses on adding the atom-free and simplicial presentation of
Chow rings of matroids. [see relevant
issue](sagemath#37987)
The former is taken from Definition 4.1 of [this
paper](https://arxiv.org/abs/2111.00393) and the latter from Definition
3.2.1 of [this paper](https://arxiv.org/pdf/1905.07114).

@tscrim

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39359
Reported by: Shriya Meenakshisundaram
Reviewer(s): Shriya Meenakshisundaram, Travis Scrimshaw
@vbraun vbraun merged commit 9159a64 into sagemath:develop Jul 25, 2025
21 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants