Implement Specht modules for diagrams #35036
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #35036 +/- ##
========================================
Coverage 88.61% 88.61%
========================================
Files 2148 2149 +1
Lines 398855 398847 -8
========================================
+ Hits 353438 353445 +7
+ Misses 45417 45402 -15
... and 121 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
I would prefer to add just one method everywhere, not the second method about rank. |
The rank computation is faster as it doesn't need to do the full echelonization. Since it can often be the point of interest (at least with more arbitrary shapes from what I've heard), I wanted to have easy access to it, plus the Specht module itself. |
| sage: s(SM.frobenius_image()) | ||
| s[2, 2, 1] | ||
| """ | ||
| from sage.combinat.specht_module import SpechtModule |
There was a problem hiding this comment.
maybe factor out a function specht_module_builder that takes a ring, a size and cells ? Instead of doing these double imports everywhere ?
There was a problem hiding this comment.
I don't really see the benefit. To me, it adds more indirection and complexity to save on just doing one import at each point of the code instead of two (as the second import is still done). To me, it is equivalent to having
class Bar:
def __init__(self, x):
self._x = x
def foo(x):
from above import Bar
return Bar(x)which I would want to just call Bar(x) within my code.
|
I don't have anything to do with releases - |
|
my advice is to rebase (rather than merge), as far as it is easy. Leads to much clearer branches. GH has an automated rebase for the base branch (instead of merging it, you can choose to rebase) |
|
@dimpase I am of the opinion rewriting history is bad (and it can mean I have to rehandle all merge conflicts; although that isn't the case here IIRC), but I will try. However, you are part of the GH transition team, and this is an issue with developing Sage with GH. |
|
@dimpase Actually, I do have merge conflicts to deal with... |
tscrim
force-pushed
the
public/combinat/specht_modules-34883
branch
from
cca8827
to
abe78e3
Compare
|
There were a number of very large edits done recently, e.g. replacing explicit references to trac.sagemath.org all over the sources. This will not happen often. |
|
The rebase seems to have fixed the GH issue, but there should have been no difference with |
|
@tscrim I didn't see what GitHub was showing in the diff; but I've just looked at your branch before the rebase (cca8827). |
|
@mkoeppe It was showing 524 files changed. I also really never want to rollback my version of Sage to a previous version just to merge in |
|
I'm aware of the problem of expensive rebuilds. I just use a separate worktree when I have to do such merges. Your fix, merging in the opposite direction, is bad practice and creates bad history. |
|
It's documented, by the way, here: https://doc.sagemath.org/html/en/developer/advanced_git.html#minimize-the-recompilation-time |
|
Okay, I see. This is a bit contrary to how I remember the development model being presented (e.g., there is no special branch). |
|
Documentation preview for this PR is ready! 🎉 |
|
in my opinion, it is not required to merge develop after getting positive review |
Quite possibly; I am somewhat of the same opinion. Although it is one click on GH and makes sure there are no merge conflicts… |
|
By merging (even better, rebasing, if possible), you're alleviating the risk that the branch does not merge cleanly any more. This is something that ought to be improved in our development workflow. IMHO most everyone thinks that our |
|
But not everyone; in particular, I am not of that opinion. Although having a more all-PRs-currently-merged branch that doesn’t change would be nice to have. Yet that might end up being more work for Volker… |
|
my point is that with other GitHub projects I contribute to, once your PR is approved, the rest is not your worry. Here, you still have to often watch your approved contribution to get all mouldy and often in need of more polish, because bitrot... |
GH checks for merge conflicts already when you view the PR; there's no need to push the merge button for that. When it displays "branch is out of date", that means it can be merged cleanly. I think the standing guidance from @vbraun still applies: Don't touch a PR that has the "positive review" label. (It may already be merged into Volker's branch.) With the modification: If GH indicates that there is a merge conflict, then it makes sense to resolve the conflicts – it typically can't be merged in this case. |
|
@mkoeppe Do the tests run against the latest develop too? @dimpase I think there is only so much that can be done for that because of a higher volume of changes that SageMath development has. The only thing I could think of to prevent that would be the every-accepted-PR-merged branch I mentioned before. Usually the biggest lag in beta versions comes around release time and just after, but I think that comes from Volker having to do too much. |
as far as I know, they are not running with an explicit trigger such as a |
as far as I know, that's what large Python orgs, such as cPython, scipy, etc., use. |
|
Merge conflict |
tscrim
force-pushed
the
public/combinat/specht_modules-34883
branch
from
fe8b956
to
ff7bc98
Compare
|
Trivial rebase. |
Fixes #34883.