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

Posets in OSCAR #3610

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

felix-roehrich
Copy link
Collaborator

This is an initial implementation for posets in OSCAR, feedback is welcome.

@lgoettgens lgoettgens added the experimental Only changes experimental parts of the code label Apr 15, 2024
Copy link
Member

@lgoettgens lgoettgens 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 some minor remarks. But what I think is way more important is to get feedback from people that actually want to use Posets in Oscar.

experimental/Posets/src/Poset.jl Outdated Show resolved Hide resolved
parent::Poset
end

function index(x::PosetElem)
Copy link
Member

Choose a reason for hiding this comment

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

The already existing index methods do something completely different. Does somebody know of other occurrences in the Oscar world of something like this and how we call it there?

Copy link
Contributor

Choose a reason for hiding this comment

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

its mostly called data to access the defining datum...

experimental/Posets/src/Poset.jl Outdated Show resolved Hide resolved
experimental/Posets/src/Poset.jl Outdated Show resolved Hide resolved

# add k to the stack and continue from k
push!(q, k)
@goto outer
Copy link
Member

Choose a reason for hiding this comment

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

I think you can just replace this goto by a break and then get rid of the @label as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I replace this with break, I will have to introduce a variable and an if statement after for loop. In the end I felt that goto is easier and the flow is clear too.

Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
@fieker
Copy link
Contributor

fieker commented Apr 15, 2024

This is not an initial implementation of posets in OSCAR, there is one in
src/NumberTheory/GaloisGrp/POSet.jl
and has been there for a long time. Is the new one supposed to replace the existing one? Can it cover everything (which is not a lot) that the old one does?
The Posets were supposed to be updated/ replaced eventually, so feel free to do so, but please have a look and see what is required in this particular application.

@felix-roehrich
Copy link
Collaborator Author

I didn't know there was an existing implementation, since it isn't exported; thanks for letting me know. The goals pursued in GaloisGrp are different from my implementation. For my application I am mostly interested in maximal chains, so I need covering relations and the edges have bonds/weights. Currently the constructor uses (and only needs) the adjacency matrix of the Hasse diagram.

@lgoettgens suggested to bring this up in the next OSCAR meeting, so other people (who need posets) can voice their needs, and then plan from there.

@micjoswig
Copy link
Member

There is a new polymake release in the making. One of its main features is substantial new functionality concerning posets. For instance, a fairly fast implementation of all maximal chains etc.

We need to align our efforts.

@fingolfin
Copy link
Member

@micjoswig is there already some kind of preliminary documentation on this somewhere, or a person to talk to about it, so that @felix-roehrich can coordinate with them?

@micjoswig
Copy link
Member

Let's discuss posets on Friday (26 April) in the regular zoom.

@micjoswig
Copy link
Member

micjoswig commented Apr 26, 2024

We will see the release of poylmake 4.12 soon (say, by May 3). That features some additions to (and renaming of) polymake's posets (which are still called lattice in 4.11 and before). It will trickle down through Polymake.jl to OSCAR.

In case you want to see this in action, just as an example, here is the polymake 4.11 code for computing an arbitrary order polytope. One nontrivial ingredient is how the maximal anti-chains are computed; this goes through a max clique computation in the comparability graph.

See also Section 5 of our preprint.

@fingolfin
Copy link
Member

Polymake 4.12 has been released now; but integration into OSCAR is still work in progress, see PR #3819.

@fingolfin fingolfin removed the triage label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Only changes experimental parts of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants