-
-
Notifications
You must be signed in to change notification settings - Fork 515
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
Reenable broken doctests in #15473 and #15476 when #10963 is merged #15475
Comments
just documentation typos that i fixed when i was reading it |
This comment has been minimized.
This comment has been minimized.
comment:1
Attachment: cat-fixes.patch.gz |
comment:2
[obsolete] |
Branch: public/combinat/15475 |
Commit: |
This comment has been minimized.
This comment has been minimized.
comment:7
Needs_review now (finally). The review is really trivial. |
Changed branch from public/combinat/15475 to public/categories/15475 |
Changed commit from |
Commit: |
Branch pushed to git repo; I updated commit sha1. New commits: |
comment:10
Branch fixed, but more bughuntery in progress. |
Changed branch from public/categories/15475 to public/combinat/15475 |
Changed branch from public/combinat/15475 to public/categories/15475 |
New commits:
|
comment:32
OUCH. I just realized that I forgot to reenable the symmetric functions doctest, and not unexpectedly it is still broken. OK, this thing must be unrelated to #10963 after all. Simon, can you make sense of this?
New commits:
|
comment:33
Replying to @darijgr:
Does that mean that the controlled C3 algorithm is not bullet proof, resp. that the global ordering imposed on the categories is not as it should be? Nicolas? |
comment:34
I'm pretty sure it's related to the category refinement that occurs for
(You can remove the
However, in a fresh Sage:
This might be useful info (also in a fresh Sage):
|
comment:35
On u/nthiery/15475-sym_in_categories_over_base_ring is a tentative workaround the MRO crash by making sym's category more independent of the base ring. Almost all tests pass in sage.combinat.sf, except for trivialities and a test involving QSym (which should probably get the same treatment). |
comment:36
Darij, would you be okay separating the current fixes of the descent and symmetric group algebras off as another ticket so we can get that in (everything up to ff4e546)? |
comment:37
Definitely. Are you also planning to fix the bugs recently posted on sage-devel? |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:41
Currently, the branch |
comment:42
Replying to @a-andre:
Yes it does, it removes the |
comment:43
Another path-dependent MRO fail related to Hopf algebras over Zmod(n) observed in #11979. This shows that we should fix the category framework or our C3, or whatever else is at fault, rather than try to change Sym into a different category. (Are we actually using our controlled C3 here, or Python's old C3?) |
comment:44
We might be able to get a good workaround by finishing #15229. |
comment:45
That's good news! #15229 might actually be a fix for the root cause, not a workaround:
and, for #11979:
So my guess would be that category refinement doesn't like it when a category changes in the midst of it, and the category of Zmod(n) does change when Zmod(n) is tested for fieldness:
With #15229 fixing this, I think we can hope to see the end of this issue. Is this pattern, where an object lazily starts off with a broad category and then refines in when the user checks for properties, common in Sage? Should we get rid of that or tweak the code to allow it? If we are to allow it, we would probably need to establish a hierarchy of categories or classes such that refining a category of some object can only trigger refinements of categories of lower-class objects in its process, and said refinements should be made sure not to interfere or run races with each other? EDIT: I also wanted to add that this whole tradeoff between "allow several instances of ZZ/nZZ with different properties, and try to somehow ensure that they behave like they are equal, e.g., providing automatic coercion between their polynomial rings" and "force all instances of ZZ/nZZ to be identical, at the cost of having to update and mutate this single instance long after it is created, and try to somehow ensure that these mutations do not break consistency" reminds me a lot of the constructivity problem of Voevodsky's univalent foundations. Looks like equality is the trickiest thing to formalize... |
comment:46
This is great to hear. I've rebased #15229, so hopefully that will get in soon so we can (finally) close this. |
comment:47
OK, this actually didn't help. :/ What I meant by #15229 fixing this is that the source of this bug was discussed in the #15229 thread. But it wasn't solved there, because with #15229 I still have:
Either this lazy discovery of categories has to be removed, or the category refinement and MRO routines must be hardened against it. |
<!-- Please provide a concise, informative and self-explanatory title. --> <!-- Don't put issue numbers in the title. Put it in the Description below. --> <!-- For example, instead of "Fixes #12345", use "Add a new method to multiply two integers" --> ### 📚 Description <!-- Describe your changes here in detail. --> <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes #12345". --> Rebased an ancient ticket branch from @darijgr: - Fixes #15475 <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [x] The title is concise, informative, and self-explanatory. - [ ] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - #12345: short description why this is a dependency - #34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #35521 Reported by: Matthias Köppe Reviewer(s): Frédéric Chapoton
This branch activates two combinat doctests which were disabled due to unexplainable bugs that were fixed in #10963:
and
Also, a few typos in the c3_controlled doc are fixed.
Disregard the attachment.
Depends on #10963
Depends on #15473
Depends on #15476
Depends on #16678
CC: @nthiery @simon-king-jena @tscrim @sagetrac-sage-combinat
Component: categories
Keywords: 10963, c3, transitivity, descent algebras, symmetric functions
Branch/Commit: public/categories/15475 @
bf34154
Issue created by migration from https://trac.sagemath.org/ticket/15475
The text was updated successfully, but these errors were encountered: