-
Notifications
You must be signed in to change notification settings - Fork 253
Update AbstractSimplicialComplexes.m2 #3821
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
base: development
Are you sure you want to change the base?
Conversation
Added options to random simplicial complex
Hi,
Yes, perhaps some additional modifications can be done along these lines—especially in light of the new randomSubsets function. However, in either case the method works well “as is” and it is able to treat both cases (at most m) vs. exactly m. This was the intent of the original method (to generically treat the at most m). Yes, if there is already a symbol “Verify” then that might be better than “Seed”. (It’s one less symbol to export.) Let me try to make some small along these lines. I’ll push these changes to the development branch again soon.
Best,
Nathan
… On May 16, 2025, at 10:50 PM, Doug Torrance ***@***.***> wrote:
@d-torrance commented on this pull request.
In M2/Macaulay2/packages/AbstractSimplicialComplexes.m2 <#3821 (comment)>:
> L := for i from 1 to n list i;
dDimlSubsets := subsets(L,d+1);
- rdmFaces := for i from 1 to m list (dDimlSubsets#(random(binomial(n,d+1))));
- append(append(rdmFaces,{L}),subsets(L,d));
- abstractSimplicialComplex(rdmFaces))
+ rdmFaces := unique(for i from 1 to m list (dDimlSubsets#(random(binomial(n,d+1)))));
I think you could use randomSubset(dDimlSubsets, m) here to guarantee that you get m distinct faces. Even better than that would be randomSubset(n, m) and use the results with an unranking algorithm to generate only the subsets you need without having to call subsets to generate a bunch of subsets you won't use.
Also, why Seed? If the option is meant to spend a bit of extra effort to verify that the output satisfies some condition, then maybe Verify would be more appropriate.
—
Reply to this email directly, view it on GitHub <#3821 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABC62JEMABK5665RFBXKNYL262PXLAVCNFSM6AAAAAB5F3Z742VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBXHA4DMNJRGQ>.
You are receiving this because you authored the thread.
|
Added "Verify" option as requested.
Hi,
I’ve pushed the changes now. Do let me know what you think. Once these are approved are you able to push these to the latest release or will this have to what until the next release? Thanks so much for these helpful comments.
Best,
Nathan
… On May 17, 2025, at 8:57 AM, Nathan Grieve ***@***.***> wrote:
Hi,
Yes, perhaps some additional modifications can be done along these lines—especially in light of the new randomSubsets function. However, in either case the method works well “as is” and it is able to treat both cases (at most m) vs. exactly m. This was the intent of the original method (to generically treat the at most m). Yes, if there is already a symbol “Verify” then that might be better than “Seed”. (It’s one less symbol to export.) Let me try to make some small along these lines. I’ll push these changes to the development branch again soon.
Best,
Nathan
> On May 16, 2025, at 10:50 PM, Doug Torrance ***@***.***> wrote:
>
>
> @d-torrance commented on this pull request.
>
> In M2/Macaulay2/packages/AbstractSimplicialComplexes.m2 <#3821 (comment)>:
>
> > L := for i from 1 to n list i;
> dDimlSubsets := subsets(L,d+1);
> - rdmFaces := for i from 1 to m list (dDimlSubsets#(random(binomial(n,d+1))));
> - append(append(rdmFaces,{L}),subsets(L,d));
> - abstractSimplicialComplex(rdmFaces))
> + rdmFaces := unique(for i from 1 to m list (dDimlSubsets#(random(binomial(n,d+1)))));
> I think you could use randomSubset(dDimlSubsets, m) here to guarantee that you get m distinct faces. Even better than that would be randomSubset(n, m) and use the results with an unranking algorithm to generate only the subsets you need without having to call subsets to generate a bunch of subsets you won't use.
>
> Also, why Seed? If the option is meant to spend a bit of extra effort to verify that the output satisfies some condition, then maybe Verify would be more appropriate.
>
> —
> Reply to this email directly, view it on GitHub <#3821 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABC62JEMABK5665RFBXKNYL262PXLAVCNFSM6AAAAAB5F3Z742VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBXHA4DMNJRGQ>.
> You are receiving this because you authored the thread.
>
|
Hi, Not sure why one check failed. Here is the install on my machine: -- i1 : uninstallPackage "AbstractSimplicialComplexes" i2 : installPackage("AbstractSimplicialComplexes", RemakeAllDocumentation => true) o2 = AbstractSimplicialComplexes o2 : Package i3 : check "AbstractSimplicialComplexes"
|
Thanks. I’ll change that.
Best,
Nathan
… On May 17, 2025, at 2:19 PM, Doug Torrance ***@***.***> wrote:
@d-torrance commented on this pull request.
In M2/Macaulay2/packages/AbstractSimplicialComplexes.m2 <#3821 (comment)>:
> -- chosen at random from all binomial(binomial(n,d+1),m) possibilities.
-- Such random complexes appear in lots of different contexts including in the article
-- Cohen-Lenstra heuristics for torsion in homology of random complexes
-- (Kahle, M. and Lutz, F. H. and Newman, A. and Parsons, K.).
-randomAbstractSimplicialComplex(ZZ,ZZ,ZZ) := AbstractSimplicialComplex => (n,m,d) -> (
- L := for i from 1 to n list i;
- dDimlSubsets := subsets(L,d+1);
- rdmFaces := for i from 1 to m list (dDimlSubsets#(random(binomial(n,d+1))));
- append(append(rdmFaces,{L}),subsets(L,d));
- abstractSimplicialComplex(rdmFaces))
+randomAbstractSimplicialComplex(ZZ,ZZ,ZZ) := AbstractSimplicialComplex => o -> (n,m,d) -> (
+ x := toList(1..n);
+ rdmFaces := unique(for i from 1 to m list randomSubset(x, d+1));
+ if o.Verify == false then abstractSimplicialComplex(rdmFaces|subsets(x,d))
o.Verify is a boolean, so there's no need to use == here. You could just do if not o.Verify then ...
—
Reply to this email directly, view it on GitHub <#3821 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABC62JC3AT3VLB27MHJS23L2654UBAVCNFSM6AAAAAB5F3Z742VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBYGM3DOOBXGE>.
You are receiving this because you authored the thread.
|
Many thanks. I will correct this!
Best,
Nathan
… On May 18, 2025, at 1:38 PM, Doug Torrance ***@***.***> wrote:
@d-torrance requested changes on this pull request.
In M2/Macaulay2/packages/AbstractSimplicialComplexes.m2 <#3821 (comment)>:
> @@ -653,17 +670,20 @@ doc ///
L = randomAbstractSimplicialComplex(6,3)
Text
Create the random complex $Y_d(n,m)$ which has vertex set
- $[n]$ and $(d − 1)$-skeleton, and has $m$ $d$-dimensional faces,
+ $[n]$ and complete $(d − 1)$-skeleton, and has exactly m d-dimensional faces,
I think this line was accidentally reverted.
In M2/Macaulay2/packages/AbstractSimplicialComplexes.m2 <#3821 (comment)>:
> @@ -653,17 +670,20 @@ doc ///
L = randomAbstractSimplicialComplex(6,3)
Text
Create the random complex $Y_d(n,m)$ which has vertex set
- $[n]$ and $(d − 1)$-skeleton, and has $m$ $d$-dimensional faces,
+ $[n]$ and complete $(d − 1)$-skeleton, and has exactly m d-dimensional faces,
chosen at random from all $\binom{\binom{n}{d+1}{m}$ possibilities.
The TeX for this binomial coefficient isn't rendering correctly -- I think you need an extra closing curly brace
—
Reply to this email directly, view it on GitHub <#3821 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABC62JAXMLUONFGLIYTEKVT27DASDAVCNFSM6AAAAAB5F3Z742VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBZGA2TAOBWHE>.
You are receiving this because you authored the thread.
|
Cosmetic changes as requested.
Yes, I decided to change the wording back to my original wording especially since there is the added functionality with the option added. I trust that is OK. Everything is entirely clear from the examples. If you truly insist, then I can make that change.
Best,
Nathan
… On May 19, 2025, at 7:42 PM, Doug Torrance ***@***.***> wrote:
@d-torrance commented on this pull request.
In M2/Macaulay2/packages/AbstractSimplicialComplexes.m2 <#3821 (comment)>:
> @@ -653,17 +670,20 @@ doc ///
L = randomAbstractSimplicialComplex(6,3)
Text
Create the random complex $Y_d(n,m)$ which has vertex set
- $[n]$ and $(d − 1)$-skeleton, and has $m$ $d$-dimensional faces,
+ $[n]$ and complete $(d − 1)$-skeleton, and has exactly m d-dimensional faces,
I think this line still needs to be fixed. The "exactly" was removed since there might be fewer than m d-dimensional faces (unless the new Verify option is set to true), but now it's back.
—
Reply to this email directly, view it on GitHub <#3821 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABC62JBNREF5T7UNIIBK3WT27JT6LAVCNFSM6AAAAAB5F3Z742VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNJSGE4DOOBTGE>.
You are receiving this because you authored the thread.
|
Cosmetic changes to the documentation as requested.
I do trust that this last change will be OK. Do let me know if any additional items are needed. |
Is there a reason that this last workflow failed? Please close this pull request and merge the files. Many thanks. |
Not to bother, but now that the build has passed, is it possible to approve the pull request and resolve the related issues/questions that were asked and motivated the implementation of this increased functionality? Many thanks in advance. Of course, it will be great if this improved functionality will appear in the next mini-M2-release. Many thanks again for your efforts with all of this. |
As I have mentioned this PR can be closed and merge. Here are a few examples to illustrate the current version of the code in the pull request to illustrate the new functionality. Just let me know if you have any additional questions. -- i2 : tally apply(1000,i->length(randomAbstractSimplicialComplex(5,3,2))_2) o2 = Tally{1 => 8 } o2 : Tally i3 : tally apply(1000,i->length(randomAbstractSimplicialComplex(5,3,2,Verify=>true))_2) o3 = Tally{3 => 1000} o3 : Tally i4 : tally apply(1000,i->length(randomAbstractSimplicialComplex(5,3,2))_1) o4 = Tally{10 => 1000} o4 : Tally -- |
Hi, Just to follow-up again, are you able to merge the PR? Do let me know if you have any additional questions and/or discussions about the newly added functionality. Many thanks. Best, |
Added options to random simplicial complex