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

Added a GAP notebook example. #5

Closed
wants to merge 4 commits into from

Conversation

rbehrends
Copy link

@rbehrends rbehrends commented Apr 27, 2020

This is a very mechanical translation of the Rubik's Cube Example, meant to address issue #3. It currently still misses text annotations and has its share of ugliness.

I also noticed that the pre-image computations gave different results, but haven't yet looked into the underlying causes.

@rbehrends rbehrends added the enhancement New feature or request label Apr 27, 2020
@rbehrends rbehrends linked an issue Apr 27, 2020 that may be closed by this pull request
@fingolfin
Copy link
Member

Thanks! Some notes:

  • we should add constructors to GAP.jl, so that e.g. BigInt(some_gap_int) works (see also Enh/refactor gap to julia GAP.jl#323)
  • I already told you about the @gap macro on Slack, you can use that here
  • we should add something like Base.in(x::GAP.GapObj, y::GAP.GapObj) = GAP.Globals.in(x, y) to GAP.jl (I'll submit a PR for that shortly)
  • once import GAPGroups Oscar.jl#84 is merged, we should make a variant of this notebook which uses it -- the two notebooks then could reference each other and be used to showcase the differences

Copy link
Member

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

I have added a few comments.
If this example is regarded as interesting, would it make sense to add also the text comments?

RubiksCubeGAP.ipynb Outdated Show resolved Hide resolved
}
],
"source": [
"f = @gap(\"FreeGroup(\\\"t\\\", \\\"l\\\", \\\"f\\\", \\\"r\\\", \\\"e\\\", \\\"b\\\")\")"
Copy link
Member

Choose a reason for hiding this comment

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

G.FreeGroup( g"t", g"l", g"f", g"r", g"e", g"b" ) is more readable.

Copy link
Author

Choose a reason for hiding this comment

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

I've actually changed that to triple quotes in the meantime, but illustrating the use of GAP strings (which I didn't know about) would be more interesting.

Copy link
Member

Choose a reason for hiding this comment

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

Yet another variant: G.FreeGroup(to_gap(["t", "l", "f", "r", "e", "b"])). All of these have pros and cons. In the end, it depends on taste.

}
],
"source": [
"pre1 = G.PreImagesRepresentative(hom, GAP.call_gap_func(@gap(\"x -> x.1\"), z) )"
Copy link
Member

Choose a reason for hiding this comment

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

For the moment, G.PreImagesRepresentative(hom, G.GeneratorsOfGroup(z)[1]) would look more natural.
The GAP syntax z.1 belongs to a special method only for groups, but if it would be regarded as interesting, one could make it available also in Julia. (I would not be in favor of this.)

Copy link
Author

Choose a reason for hiding this comment

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

Hmm. I've figured out in the meantime that GAP functions can actually be called directly, e.g @gap("x -> x + 1")(z); this might be illustrative, but I think your approach is more idiomatic. I'll think about that.

Copy link
Member

Choose a reason for hiding this comment

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

You can also just write pre1 = G.PreImagesRepresentative(hom, z.:1)

@rbehrends
Copy link
Author

I can't judge myself whether this is useful for a wider audience; I simply picked the first large example on the GAP website and adapted it. I figured that even if it is not of interest for the examples on the OSCAR website (which I think is more about adding examples relevant to the overall TRR project), it can still serve educational purposes.

I'll be happy to adapt the comments from the original code if people think this example is useful. I simply wanted to get something workable out first.

@fingolfin
Copy link
Member

I think it's useful, and adding the comments would be good.

@rbehrends
Copy link
Author

I've added descriptive text and added some more comments on the specifics of using this with GAP.jl. The notebook can be viewed here directly.

@fingolfin
Copy link
Member

@rbehrends Can you please update this, now that GAP.jl 0.4.0 is out (so e.g. in is now available)

@fingolfin
Copy link
Member

Also you shouldn't need big_int anymore, big should work instead

@fingolfin fingolfin closed this Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a GAP notebook
3 participants