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.

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

Problem with orientations of simplices in simplicial complex maps. #36849

Merged

Conversation

jhpalmieri
Copy link
Member

Problem with orientation of simplices in simplicial complex maps

In rare cases, Sage incorrectly computes the chain complex map associated to a map of simplicial complexes. The problem is that the orientation of each simplex in the image is not correctly compared to its actual orientation in the codomain: an edge (f(v), f(w)) is not compared correctly to see whether the actual simplex in the codomain is stored as (f(v), f(w)) or (f(w), f(v)).

I think that the default sorting of vertices (and hence simplices) means that this usually doesn't come up, but it can arise when the vertices do not have an obvious ordering. See https://ask.sagemath.org/question/74754/chain-morphism-between-subdivisions/ for one such case.

📝 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.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

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.

LGTM.

Although if you want to make one change, please add a reference to this PR in the TESTS: block. Once you do that or decide not to, you can change this to a positive review.

@jhpalmieri
Copy link
Member Author

Thanks very much. I'll make the change you suggested soon and then change the status.

@tscrim
Copy link
Collaborator

tscrim commented Dec 13, 2023

Thanks.

Copy link

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

@vbraun vbraun merged commit bfb861b into sagemath:develop Dec 19, 2023
16 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants