Opened 12 years ago

Closed 7 years ago

#10194 closed enhancement (fixed)

Set factories

Reported by: Nicolas M. Thiéry Owned by: Florent Hivert
Priority: major Milestone: sage-6.8
Component: combinatorics Keywords: factories, days30, Cernay2012, days57
Cc: Sage Combinat CC user, Florent Hivert Merged in:
Authors: Florent Hivert Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 763f93c (Commits, GitHub, GitLab) Commit: 763f93c95ecf8e79ff48339491212a6417d2c4f8
Dependencies: Stopgaps:

Status badges

Description (last modified by Frédéric Chapoton)

At Sage days 30, a long brainstorm seems to have finalized the design. Here is a excerpt from the documentation:

A set factory F is device, whose goal is to construct parent P which models subsets of a big set S. Typically, the P s are constructed within families obtained by putting a bunch of constraints cons on the elements of the set S. In such a hierarchy of subsets, one needs to have a fine and easy control on the elements construction. That is, one often needs that P constructs elements in a subclass of its usual class for element. On the contrary, one also often needs P to be a facade parent, meaning that P construct element whose actual parent is not P itself.

The role of a set factory is twofold:

  • manage a database of constructors for the different parents P = F(cons) depending on the various kinds of constraints cons. Note: currently there is no real support for that. We are gathering use case before fixing the interface.
  • ensure that the elements e = P(...) created by the different parents follows a consistent policy concerning their class and parent.

The patch implement this idea while trying to leave as much as possible space for further improvement. In particular, I tried to specify the few possible things about constraints. I'm even not completely sure about add_constraints. Please comment and review.

Florent

Attachments (2)

trac_10194-factories_policy-fh.patch (57.1 KB) - added by Florent Hivert 11 years ago.
trac_10194-factories_policy-fh-v2.patch (57.2 KB) - added by Frédéric Chapoton 9 years ago.

Download all attachments as: .zip

Change History (69)

comment:1 Changed 12 years ago by Nicolas M. Thiéry

Patch under development on the Sage-Combinat patch server.

comment:2 Changed 12 years ago by Florent Hivert

Description: modified (diff)
Owner: changed from Sage Combinat CC user to Florent Hivert
Summary: Enumerated set factoriesSet factories

comment:3 Changed 12 years ago by Florent Hivert

Keywords: factories days30 added

comment:4 Changed 11 years ago by Florent Hivert

Keywords: Cernay2012 added

comment:5 Changed 11 years ago by Florent Hivert

Milestone: sage-5.0

comment:6 Changed 11 years ago by Florent Hivert

Description: modified (diff)
Status: newneeds_review

Changed 11 years ago by Florent Hivert

comment:7 Changed 11 years ago by David Loeffler

Status: needs_reviewneeds_work
Work issues: doctest failures

This fails doctests on every version tested -- see patchbot logs

comment:8 in reply to:  7 Changed 11 years ago by Florent Hivert

Dependencies: #10963
Status: needs_workneeds_review

Replying to davidloeffler:

This fails doctests on every version tested -- see patchbot logs

I forgot a dependency to #10963. Thanks for pointing this out.

comment:9 Changed 11 years ago by David Loeffler

Status: needs_reviewneeds_info

There is no patch at #10963. I don't think this can be meaningfully tagged as "needs review" when one of its dependencies doesn't exist yet.

comment:10 in reply to:  9 Changed 11 years ago by Florent Hivert

Replying to davidloeffler:

There is no patch at #10963. I don't think this can be meaningfully tagged as "needs review" when one of its dependencies doesn't exist yet.

As you can see on #10963, the patch is far for being inexistent as Simon and Christian started to review it. Of course it is on the Sage-Combinat queue. I need to ping Nicolas to have him finish it. Now, I don't see why having Nicolas's patch unfinished forbid me to have some feedback on this one. And asking for such a feedback seems to be a perfectly meaningful reason for putting "needs review"... The only reason I can see for not doing it is that it can trouble the patchbot. Do you have another one ?

comment:11 Changed 11 years ago by Nicolas M. Thiéry

Salut Florent!

Why is there a dependency on 10963? In the queue, this patch is much lower than 10963.

comment:12 Changed 11 years ago by Nicolas M. Thiéry

Ah, I see, it's about the glitch Sets().Facade() <-> Sets().Facades().

Since anyway the patch is up the queue, I suggest we finish commuting the two patches. Just use Sets().Facades() (we will need to keep the backward compatibility anyway).

With that, the tests pass up to to trivially failing doctests:

File "/opt/sage-5.0.beta11/devel/sage-combinat/sage/structureset_factories.py", line 644:
    sage: P2.category()
Expected:
    Join of Category of facade finite enumerated sets and Policy category with facade parent AllPairs
Got:
    Join of Category of finite enumerated sets and Policy category with facade parent AllPairs
**********************************************************************
File "/opt/sage-5.0.beta11/devel/sage-combinat/sage/structure/set_factories.py", line 972:
    sage: P.category()
Expected:
    Join of Category of facade finite enumerated sets and Policy category with facade parent AllPairs
Got:
    Join of Category of finite enumerated sets and Policy category with facade parent AllPairs

Cheers,

comment:13 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

Changed 9 years ago by Frédéric Chapoton

comment:14 Changed 9 years ago by Frédéric Chapoton

new version, rebased on 5.12.beta0, with cleanup

comment:15 Changed 9 years ago by Frédéric Chapoton

Branch: u/chapoton/10194
Commit: 5dbd72b43c05ae78ddafb670ddc2cd31f06575a7

I have made a git branch, but it seems to be massively broken.

Some problems come from Sets().Facade() versus Sets().Facades() as explained before

Some other problems come from #14519

And some other problems I do not understand..


New commits:

5dbd72btrac_10194-factories_policy-fh.patch

comment:16 Changed 9 years ago by git

Commit: 5dbd72b43c05ae78ddafb670ddc2cd31f06575a757f3ed3e2e888dffc5c1a860b8eb47662c7333e0

Branch pushed to git repo; I updated commit sha1. New commits:

57f3ed3trac #10194 minor changes

comment:17 Changed 9 years ago by git

Commit: 57f3ed3e2e888dffc5c1a860b8eb47662c7333e028c42cb58e478741a0bad333b853c7f2aea86468

Branch pushed to git repo; I updated commit sha1. New commits:

28c42cbtrac #10194 typo corrections

comment:18 Changed 9 years ago by Frédéric Chapoton

Dependencies: #10963

Temporary : emptied dependency field (was #10963)

comment:19 Changed 9 years ago by git

Commit: 28c42cb58e478741a0bad333b853c7f2aea864686fa45d8a4c031d612966f336fbfc7be992911b14

Branch pushed to git repo; I updated commit sha1. New commits:

6fa45d8trac #10194 other details, still not working :(

comment:20 Changed 9 years ago by Frédéric Chapoton

Status: needs_infoneeds_work

This needs to be worked upon, as it is not working at all.

comment:21 Changed 9 years ago by Frédéric Chapoton

Maybe some of the problems come from #13801 ?

comment:22 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:23 Changed 9 years ago by Florent Hivert

Branch: u/chapoton/10194u/hivert/10194

comment:24 Changed 9 years ago by git

Commit: 6fa45d8a4c031d612966f336fbfc7be992911b1406beedce649e34107358217e13ef69bd31752ac5

Branch pushed to git repo; I updated commit sha1. New commits:

06beedcFixed passing parent to ElementWrapper

comment:25 Changed 9 years ago by nborie

Keywords: days57 added

comment:26 Changed 9 years ago by git

Commit: 06beedce649e34107358217e13ef69bd31752ac5d7eb4c1317c4bc5a0d33312879d2938cc54e5d25

Branch pushed to git repo; I updated commit sha1. New commits:

d7eb4c1Cleanup in constraints managment.

comment:27 Changed 9 years ago by git

Commit: d7eb4c1317c4bc5a0d33312879d2938cc54e5d25b615150f459ea3f520cdc190325e961d3ce9ad22

Branch pushed to git repo; I updated commit sha1. New commits:

b615150Fixed printing of _repr_ for TopMostParentPolicy

comment:28 Changed 9 years ago by Florent Hivert

Status: needs_workneeds_review

comment:29 Changed 9 years ago by Frédéric Chapoton

Maybe you should add #13580 as a dependency ?

comment:30 in reply to:  29 Changed 9 years ago by Florent Hivert

Replying to chapoton:

Maybe you should add #13580 as a dependency ?

Why ? Those are completely different things.

comment:31 Changed 9 years ago by Frédéric Chapoton

Then why do you have that in the commits of this ticket ?

comment:32 in reply to:  31 Changed 9 years ago by Florent Hivert

Replying to chapoton:

Then why do you have that in the commits of this ticket ?

I probably screwed things up with git ! I'm trying to fix the problem.

comment:33 Changed 9 years ago by nborie

For information, this features is largely tested in #16094

comment:34 Changed 9 years ago by Florent Hivert

Branch: u/hivert/10194u/hivert/10194-rebased

comment:35 Changed 9 years ago by Florent Hivert

Commit: b615150f459ea3f520cdc190325e961d3ce9ad22fc6981b5ac4cf52c86973288c900ac08f8511042

Should be fixed now ! But I had to rebase the branch. So Nicolas, you will have to rebase yours at some point. Sorry !


New commits:

dc5dfc8# Mon Apr 07 00:47:29 2014 +0200
e595280Fixed passing parent to ElementWrapper
80b08c3Cleanup in constraints managment.
fc6981bFixed printing of _repr_ for TopMostParentPolicy

comment:36 Changed 9 years ago by Florent Hivert

Branch: u/hivert/10194-rebasedu/hivert/10194-rebased-beta7

comment:37 Changed 9 years ago by nborie

Branch: u/hivert/10194-rebased-beta7u/nborie/10194-rebased-beta7

comment:38 Changed 9 years ago by Frédéric Chapoton

Branch: u/nborie/10194-rebased-beta7public/ticket/10194
Commit: fc6981b5ac4cf52c86973288c900ac08f85110420eeecd1c94859b2328a1487e4d5dae91b158e5cb

I have made one more commit. Now all tests pass, and code is in the pep8 standard.


New commits:

77279f9# Mon Apr 07 00:47:29 2014 +0200
272d17fFixed passing parent to ElementWrapper
1491bffCleanup in constraints managment.
41c137eFixed printing of _repr_ for TopMostParentPolicy
f76e0e6Review : fonctionality, documentations, tests (very small things remains...)
1f3e02bMerge branch 'u/nborie/10194-rebased-beta7' of trac.sagemath.org:sage into 10194
0eeecd1trac #10194 make sure that all test pass (and pyflakes and pep8 compliant)

comment:39 Changed 9 years ago by Frédéric Chapoton

There remains a pyflakes warning thta needs to be taken care of :

src/sage/structure/set_factories.py:722: undefined name 'FacadeParentPolicyCategory'

comment:40 Changed 9 years ago by Frédéric Chapoton

Status: needs_reviewneeds_work
Work issues: doctest failurespyflakes warning

comment:41 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:42 Changed 9 years ago by git

Commit: 0eeecd1c94859b2328a1487e4d5dae91b158e5cb16ca51901cd8a266b3ae59a90425deedd270d0a6

Branch pushed to git repo; I updated commit sha1. New commits:

f823667Minor doc improvements
16ca519Merge branch 'public/ticket/10194' of trac.sagemath.org:sage into t/10194/factories_policy

comment:43 Changed 9 years ago by Frédéric Chapoton

what about the pyflakes warning ? could you take care of that please ?

comment:44 Changed 9 years ago by git

Commit: 16ca51901cd8a266b3ae59a90425deedd270d0a6014d842b9917c3090619cc9ea6aac681dde5c0bb

Branch pushed to git repo; I updated commit sha1. New commits:

364a0c0Merge branch 'public/ticket/10194' of ssh://trac.sagemath.org:22/sage into 10194
014d842trac #10194 details and formatting

comment:45 Changed 8 years ago by git

Commit: 014d842b9917c3090619cc9ea6aac681dde5c0bbe061484adfe23b2b8c2a5d0a1fafe4cff1de2b5d

Branch pushed to git repo; I updated commit sha1. New commits:

4895320Merge branch 'public/ticket/10194' of ssh://trac.sagemath.org:22/sage into 10194
e061484trac #10194 removed the category method (was useless and wrong ?)

comment:46 Changed 8 years ago by Frédéric Chapoton

Status: needs_workneeds_review
Work issues: pyflakes warning

comment:47 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:48 Changed 8 years ago by Frédéric Chapoton

At last, got a green report from the bot. Florent, do you think that this is ready ?

comment:49 Changed 8 years ago by Frédéric Chapoton

Reviewers: Frédéric Chapoton

comment:50 Changed 8 years ago by git

Commit: e061484adfe23b2b8c2a5d0a1fafe4cff1de2b5d87a475cdfa8d9cdd4ca6d3c4d337fd71570f3004

Branch pushed to git repo; I updated commit sha1. New commits:

87a475cMerge branch 'public/ticket/10194' of ssh://trac.sagemath.org:22/sage into 10194

comment:51 Changed 8 years ago by git

Commit: 87a475cdfa8d9cdd4ca6d3c4d337fd71570f3004641c28adea6a0ff81aeab6cf4a928e8c45613d6d

Branch pushed to git repo; I updated commit sha1. New commits:

641c28atrac #10194 minor doc changes

comment:52 Changed 8 years ago by git

Commit: 641c28adea6a0ff81aeab6cf4a928e8c45613d6ded7f340783a1850d309af930f2fab698295482b9

Branch pushed to git repo; I updated commit sha1. New commits:

3c12f5fMerge with 6.3.beta4
ed7f340trac #10194 found a few typo again

comment:53 Changed 8 years ago by Frédéric Chapoton

@Florent : What is the meaning of the sentences

Put a nice warning _single_pair

and

Comment that and put link to documentation caveat....

in the example file ?

comment:54 Changed 8 years ago by git

Commit: ed7f340783a1850d309af930f2fab698295482b920f17b3db7974884ec741d310638511574044b96

Branch pushed to git repo; I updated commit sha1. New commits:

20f17b3Merge branch 'public/ticket/10194' of trac.sagemath.org:sage into 6.4.b6

comment:55 Changed 8 years ago by Frédéric Chapoton

Status: needs_reviewneeds_info

@Florent : PING, cf comment 53

comment:56 Changed 8 years ago by git

Commit: 20f17b3db7974884ec741d310638511574044b9652550e0f8014e4fd0c32365426c4de3ddb0a36e5

Branch pushed to git repo; I updated commit sha1. New commits:

52550e0Merge branch 'develop' into t/10194/public/ticket/10194

comment:57 Changed 8 years ago by Frédéric Chapoton

9 failing doctests, see patchbot report

comment:58 Changed 8 years ago by git

Commit: 52550e0f8014e4fd0c32365426c4de3ddb0a36e5ab1507c0dc9386b8619cd098c1710ab9bd3de50c

Branch pushed to git repo; I updated commit sha1. New commits:

ab1507cRe-reading set factories

comment:59 in reply to:  57 Changed 8 years ago by Florent Hivert

Replying to chapoton:

9 failing doctests, see patchbot report

Hi Frederic,

I should have fixed most of the problems (including comment 53). I still need a last pass of re-reading before putting it as needs review.

Florent

comment:60 Changed 8 years ago by git

Commit: ab1507c0dc9386b8619cd098c1710ab9bd3de50c586248e263a81254fa5395676d6cd7a1906bba12

Branch pushed to git repo; I updated commit sha1. New commits:

586248eVarious cleanup in the documentation

comment:61 Changed 8 years ago by Florent Hivert

Status: needs_infoneeds_review

This should be ready for review

Florent

comment:62 Changed 8 years ago by git

Commit: 586248e263a81254fa5395676d6cd7a1906bba12763f93c95ecf8e79ff48339491212a6417d2c4f8

Branch pushed to git repo; I updated commit sha1. New commits:

763f93cMerge tag '6.7' into t/10194/factories_policy

comment:63 Changed 8 years ago by Frédéric Chapoton

Milestone: sage-6.4sage-6.8

Was there any need for a merge ? By the way, I usually do the merge in the other direction. This gives much smaller commits, as you can see in the commit list.

Last edited 8 years ago by Frédéric Chapoton (previous) (diff)

comment:64 in reply to:  63 Changed 8 years ago by Florent Hivert

Replying to chapoton:

Was there any need for a merge ? By the way, I usually do the merge in the other direction. This gives much smaller commits, as you can see in the commit list.

No there wasn't, but my goal is to update trees to use factories and ultimately to do computation in operads. As you merged 6.7 in operads I also merged this one to avoid going back and forth.

Version 0, edited 8 years ago by Florent Hivert (next)

comment:65 Changed 7 years ago by Frédéric Chapoton

Description: modified (diff)
Status: needs_reviewpositive_review

ok, let this get in. There are several combinatorial tickets on top of it.

comment:66 Changed 7 years ago by Frédéric Chapoton

Description: modified (diff)

comment:67 Changed 7 years ago by Volker Braun

Branch: public/ticket/10194763f93c95ecf8e79ff48339491212a6417d2c4f8
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.