Opened 6 years ago
Closed 6 years ago
#16598 closed enhancement (fixed)
New empty design classes for a better user interface and new is_group_divisible_design Cython function
Reported by:  ncohen  Owned by:  ncohen 

Priority:  major  Milestone:  sage6.4 
Component:  combinatorial designs  Keywords:  
Cc:  vdelecroix  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  5f1fb98 (Commits)  Commit:  5f1fb98b5a72459b220b87f1c902bda62e4a166a 
Dependencies:  #16553, #16766  Stopgaps: 
Description
This branch defines:
GroupDivisibleDesign
PairwiseBalancedDesign
BalancedIncompleteBlockDesign
TransversalDesign
All these classes (which inherit from IncidenceStructure
) make it easier to do things like compute the automorphism_group
of a transversal design.
The branch also adds a function is_group_divisible_design
in designs_pyx.pyx
, and an empty is_pairwise_balanced_design
that call it. We can thus get rid of the ugly _check_pbd
.
Nathann
Change History (54)
comment:1 Changed 6 years ago by
 Branch set to u/ncohen/16553
 Commit set to ff72248b89697ebc63ce170e600b3b7764c7c178
 Dependencies set to #16553
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Branch changed from u/ncohen/16553 to u/ncohen/16598
 Commit changed from ff72248b89697ebc63ce170e600b3b7764c7c178 to 1575f3131e0663b699d9c0a35b0afb88912d7a7a
New commits:
4dc250a  trac #16553: Clean IncidenceStructure

bd609fc  trac #16553: is_t_design

11994ef  trac #16553: Review

e7a97ea  trac #16553: doc fix + deprecation is_block_design

3c0dd71  trac #16553v3: change .points() > .ground_set()

52b7177  trac #16553: merge sage 6.3.beta5

0698433  trac #16553: deprecated alias .points() + fix

1575f31  trac #16598: Useless new classes and a replacement for _check_pbd

comment:3 Changed 6 years ago by
 Commit changed from 1575f3131e0663b699d9c0a35b0afb88912d7a7a to cdaf1e2e834f3895d1fb096a139446bf8620d319
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
cdaf1e2  trac #16598: Useless new classes and a replacement for _check_pbd

comment:4 followup: ↓ 6 Changed 6 years ago by
 Status changed from needs_review to needs_work
Hey Nathann,
In group divisible design, there must be more parameters:
v
the size of the ground setK
the admissible size of the blocksG
the admissible size of the groups
The definition seems to be standard (see the big paper of Hanani and the definition IV.1.3 of the Handbook)
Anyway, GDD is just a strict generalization of PBD and it is very clear from Handbook IV.1.3.
That way, your is_pbd
would just be a lambda function.
Actually, it would be nicer that IncidenceStructure
would actually be a GroupDivisibleDesign
(groups being the partition into points). The points can be ordered in such way that the groups are
0 1 ... g11g1 g1+1 ... g1+g21...
and then the new class would just need an extra ._group_sizes
attribute that would be an integer partition.
Some methods are not well adapted to group divisible designs
dual
: is the dual well defined for GDD?incidence_matrix
: must be the incidence matrix group/block (or put a flag allowing to have points/blocks)automorphis_group
is wrong since it must preserver the groups
Why do you use tuples where everywhere in IncidenceStructure
there are lists?
Would you mind having a "stupid" .group_sizes()
method?
Vincent
comment:5 Changed 6 years ago by
Note that a solution for computing the automorphism group is as follows:
 compute the automorphism group of the partition into groups (the generators can be provided by hand, and I can provide a function for it)
 compute the automorphism group of the block design (ie forget about the groups)
 make the intersection
I think that it is not that bad in terms of efficiency.
Vincent
comment:6 in reply to: ↑ 4 ; followup: ↓ 9 Changed 6 years ago by
Yo !
In group divisible design, there must be more parameters:
v
the size of the ground setK
the admissible size of the blocksG
the admissible size of the groupsThe definition seems to be standard (see the big paper of Hanani and the definition IV.1.3 of the Handbook)
Seems more than we need at the moment, but okay ...
Anyway, GDD is just a strict generalization of PBD and it is very clear from Handbook IV.1.3. That way, your
is_pbd
would just be a lambda function.
lambda functions have no documentation. But yes, the code just takes one line.
Actually, it would be nicer that
IncidenceStructure
would actually be aGroupDivisibleDesign
(groups being the partition into points). The points can be ordered in such way that the groups are0 1 ... g11g1 g1+1 ... g1+g21...and then the new class would just need an extra
._group_sizes
attribute that would be an integer partition.
Until we want GDD to be defined on something different than 0,...,n1
just like you did for IncidenceStructure
Some methods are not well adapted to group divisible designs
dual
: is the dual well defined for GDD?
Given that the GDD is a set of blocks, why not ? Plus you wanted two lines above that GDD be equal to an IncidenceStructure
.
incidence_matrix
: must be the incidence matrix group/block (or put a flag allowing to have points/blocks)
Why ?
automorphis_group
is wrong since it must preserver the groups
Then the implementation is correct.
Why do you use tuples where everywhere in
IncidenceStructure
there are lists?Would you mind having a "stupid"
.group_sizes()
method?
Yes, I hate those methods. Write it if you want one.
Nathann
comment:7 Changed 6 years ago by
 Commit changed from cdaf1e2e834f3895d1fb096a139446bf8620d319 to 3f8cebb0d68d7e7ae0e9c763e9bc78a1bdb847dd
Branch pushed to git repo; I updated commit sha1. New commits:
3f8cebb  trac #16598: G and K as arguments of GDD

comment:8 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:9 in reply to: ↑ 6 Changed 6 years ago by
Then the implementation is correct.
i.e. an automorphism send a noncovered pair of vertices u,v on another noncovered pair of vertices u,v. So a group is sent on another group with the same cardinality.
It corresponds to the notion of automorphism group you expect for BIBD (K=[1]
), and it also corresponds to the notion of group that you expect on transversal designs (K=[k]
).
Nathann
comment:10 followup: ↓ 11 Changed 6 years ago by
 Status changed from needs_review to needs_info
I got it for the automorphism group! Thanks. But your definition of gdd is wrong: any pair of points is either in a group or in lambda blocks but not both. And this is why the blocks completely determine the groups: two points are in the same group if they are not in a block!
What do you think of having only one class (with blocks and groups) and methods:
.is_t_design(self,t,v,k,l)
.is_group_divisible_design(self,v,G,K,l)
.is_partially_balanced_design(self,v,K,l)
I do not like the fact of having one extra empty class whose only purpose is a check at creation... Moreover, if we switch to mutable classes it would be cool that after transformations we check the kind of thing we obtain. The only thing that bother me a little bit is this dual
method which does not make any sense for groups (perhaps it does??).
Why do you not translate the groups over integers as it is the case for blocks?
You really like to use tuple of tuples for groups and list of lists for blocks (see comment:4 above)?
Vincent
comment:11 in reply to: ↑ 10 Changed 6 years ago by
 Status changed from needs_info to needs_review
Hello !
I got it for the automorphism group! Thanks. But your definition of gdd is wrong:
Ahahahahah. Well, that's what I intended, but... is the new version clearer ?
What do you think of having only one class (with blocks and groups) and methods:
.is_t_design(self,t,v,k,l)
.is_group_divisible_design(self,v,G,K,l)
.is_partially_balanced_design(self,v,K,l)
How would you call the class ? If you want to have these functions as methods of something, the only choice I see is IncidenceStructure
. We would have no .groups()
method but those functions could get the groups as an argument.
I do not like the fact of having one extra empty class whose only purpose is a check at creation...
I just created GDD because it was the common generalization of TD
and PBD
. Mostly for the doc at the moment, and we will have some place to put code that works on groups.
Moreover, if we switch to mutable classes it would be cool that after transformations we check the kind of thing we obtain.
Hmmmm... I don't expect that GDD or anything that inherits from it will ever be mutable. IncidenceStructure
could be mutable someday, though. It wouldn't be cool to have a GDD object which is not a GDD. Plus as you noticed, most of these objects are created from others, rarely modified inplace.
The only thing that bother me a little bit is this
dual
method which does not make any sense for groups (perhaps it does??).
Well, it just does not consider them. To me those "groups" are a parameter of the design, nothing else... The dual of a dual of a GDD is a GDD, so everything is cool. The blocks are forgotten (they can be recomputed automatically anyway), but who cares ? I don't think that you would find any other definition of the dual of a GDD anywhere...
Why do you not translate the groups over integers as it is the case for blocks?
Oh. I thought that I would only define GDD for integer ground set, but indeed it also handles the noninteger case almost for free as we know that "._blocks" are already translated to integers. You are right. I will updated that in a second.
You really like to use tuple of tuples for groups and list of lists for blocks (see comment:4 above)?
I don't understand. .groups()
returns lists at the moment (before my commit)
def groups(self): return map(list,self._groups)
Oh, I see ! You meant for internal storage ! Fixed in the new commit.
Nathann
P.S. : While writing the commit, I made IncidenceStructure mutable. But it's not reaaaaaaaaaaallly a problem, it preserves all the structure ! :D
comment:12 Changed 6 years ago by
 Commit changed from 3f8cebb0d68d7e7ae0e9c763e9bc78a1bdb847dd to fa3c715732a965786f86a2a0ce7087c63294519a
Branch pushed to git repo; I updated commit sha1. New commits:
fa3c715  trac #16598: Relabel, review, doc fix, ...

comment:13 followup: ↓ 14 Changed 6 years ago by
Hi Nathann,
Groups are not a parameter. If you have a GDD, the group are completely determined by the blocks: define the groups in such way that two points are in the same group if and only if they do not belong to a common block.
So, it makes sense to have .is_gdd()
at the level of IncidenceStructure
and make it returns the groups if needed. We can also have a method .groups()
on IncidenceStructure
without any attribute ._groups
defined.
What do you think?
Vincent
comment:14 in reply to: ↑ 13 ; followup: ↓ 15 Changed 6 years ago by
Groups are not a parameter.
v and k are not a parameter of a BIBD either, they can be deduced. Same for K in PBD.
If you have a GDD, the group are completely determined by the blocks: define the groups in such way that two points are in the same group if and only if they do not belong to a common block.
I know, I mentionned it in my last comment....
So, it makes sense to have
.is_gdd()
at the level ofIncidenceStructure
and make it returns the groups if needed. We can also have a method.groups()
onIncidenceStructure
without any attribute._groups
defined.What do you think?
I don't like it, you cannot define "groups" properly at this level. What do you think the "groups" of IncidenceStructure(3,[[1,2]])
should be ?
Nathann
comment:15 in reply to: ↑ 14 ; followup: ↓ 16 Changed 6 years ago by
Replying to ncohen:
So, it makes sense to have
.is_gdd()
at the level ofIncidenceStructure
and make it returns the groups if needed. We can also have a method.groups()
onIncidenceStructure
without any attribute._groups
defined.What do you think?
I don't like it, you cannot define "groups" properly at this level. What do you think the "groups" of
IncidenceStructure(3,[[1,2]])
should be ?
All right, add the extra assumption that each point is covered by at least one block! If not .groups()
raises an exception.
comment:16 in reply to: ↑ 15 ; followup: ↓ 17 Changed 6 years ago by
All right, add the extra assumption that each point is covered by at least one block! If not
.groups()
raises an exception.
It's unrelated. What are the blocks of IncidenceStructure(4,[[0,1],[1,2],[2,3]])
? I am just telling you that {"u~v" if the pair u,v
is not covered by a block} is not an equivalence relation unless you are in a GDD.
Nathann
comment:17 in reply to: ↑ 16 ; followup: ↓ 18 Changed 6 years ago by
Replying to ncohen:
All right, add the extra assumption that each point is covered by at least one block! If not
.groups()
raises an exception.It's unrelated. What are the blocks of
IncidenceStructure(4,[[0,1],[1,2],[2,3]])
? I am just telling you that {"u~v" if the pairu,v
is not covered by a block} is not an equivalence relation unless you are in a GDD.
For GDD you have an extra assumption of the existence of a lambda:
blocks = [[0,1],[0,2]] groups = [[0],[1,2]]
is not a GDD (0 is covered by two blocks, but 1 and 2 are covered by one block). But "not being in a block" for pairs is an equivalence relation...
I definitely agree that "not being in the same block" is not an equivalence relation for many incidence structures (lack of transitivity)... but it is well defined and symmetric.
Vincent
comment:18 in reply to: ↑ 17 Changed 6 years ago by
For GDD you have an extra assumption of the existence of a lambda:
blocks = [[0,1],[0,2]] groups = [[0],[1,2]]is not a GDD (0 is covered by two blocks, but 1 and 2 are covered by one block). But "not being in a block" for pairs is an equivalence relation...
Since when do you need to be regular to be a GDD ? Even PBD are not necessarily regular ! Lambda is about pairs of vertices.
Nathann
comment:19 Changed 6 years ago by
 Commit changed from fa3c715732a965786f86a2a0ce7087c63294519a to 1a5a4b8203a2ad104d617a150e2029dbe3a315b1
Branch pushed to git repo; I updated commit sha1. New commits:
1a5a4b8  trac #16598: The constructors implicitly assumed that the ground set was made of integers

comment:20 followup: ↓ 21 Changed 6 years ago by
 Status changed from needs_review to needs_work
Hi Nathann,
I added a little commit of cleaning (and also replace the deepcopy by something more explicit) at u/vdelecroix/16598
.
The semantic of copy=True/False
is currently not the same with .blocks()
and .groups()
. I decided (and you did not complain) that with copy=False
it just returns a pointer to the attribute ._blocks
which means something on the integers... whatever the ground set could be. If you think it is bad, then we need to change it in .blocks()
and .points()
. Otherwise, change it in .groups()
.
In the function is_group_divisible_design
it is assumed that the ground set is {0, ..., v1}
(which is just find since it is called with the internal data from GroupDivisibleDesign
). It would be cool to specify it in the doc...
What is the point of matrix[i*n+j] != <unsigned short> 1
(in is_group_divisible_design
)?
Vincent
comment:21 in reply to: ↑ 20 Changed 6 years ago by
 Status changed from needs_work to needs_review
I added a little commit of cleaning (and also replace the deepcopy by something more explicit) at
u/vdelecroix/16598
.
Looks good !
The semantic of
copy=True/False
is currently not the same with.blocks()
and.groups()
. I decided [...]
Oh, cool ! Actually I thought it was written the way I first wrote .groups
and I thought that it was not the correct way to implement it, and so I implemented it this way to follow what I thought was the previou choice. I prefer it this way too ! Fixed.
In the function
is_group_divisible_design
it is assumed that the ground set is{0, ..., v1}
(which is just find since it is called with the internal data fromGroupDivisibleDesign
). It would be cool to specify it in the doc...
It was specified in the documentation of the mandatory argument v
, but I added another occurrence in the oneline description of the function
What is the point of
matrix[i*n+j] != <unsigned short> 1
(inis_group_divisible_design
)?
Leftover from a previous implementation. Removed !
Nathann
comment:22 Changed 6 years ago by
 Commit changed from 1a5a4b8203a2ad104d617a150e2029dbe3a315b1 to 494d82657000af8d4361a630035227c6aab6f5de
comment:23 Changed 6 years ago by
 Summary changed from Useless new classes and a replacement for _check_pbd to Useful classes for designs and a replacement for _check_pbd
comment:24 Changed 6 years ago by
O_o
comment:25 Changed 6 years ago by
 Summary changed from Useful classes for designs and a replacement for _check_pbd to Useless new design classes and a replacement for _check_pbd
Hey man, I still name my tickets how I see fit, can't I ? O_o
comment:26 followup: ↓ 28 Changed 6 years ago by
Of course you can  you just did it and the system didn't stop you, since it's brainless.
This is however not your ticket, it is a ticket (authored by you) intended to be merged into a public collaborative project, where it is helpful to have meaningful titles/descriptions/commits. If the title is reflective of this ticket, I am ready to set it to positive review as invalid/wontfix.
comment:27 followup: ↓ 29 Changed 6 years ago by
Hopefully, humans are not computers and they do understand humour... if you base your reviews on (first degree interpretation of) title of tickets I am not sure it is only the system which is brainless.
comment:28 in reply to: ↑ 26 Changed 6 years ago by
It seems that one cannot have an enjoyable conversation with anybody, these days ... I don't get it. It cannot be the weather, it is summer !
Nathann
comment:29 in reply to: ↑ 27 ; followup: ↓ 30 Changed 6 years ago by
Replying to vdelecroix:
Hopefully, humans are not computers and they do understand humour... if you base your reviews on (first degree interpretation of) title of tickets I am not sure it is only the system which is brainless.
Yes, and when these smart humans look at changelogs to get a sense of new functionality, of course they don't stop there: they open each ticket, read the description, the discussion, and the source code of the branch. And who cares about commit messages when it is so easy to look at diff for anything and make your own judgment unbiased by authors humour?
Or perhaps I am wrong all the way from the start and nobody cares about new functionality or history, so it absolutely does not matter what one can or cannot see in changelogs.
comment:30 in reply to: ↑ 29 ; followup: ↓ 31 Changed 6 years ago by
Or perhaps I am wrong all the way from the start and nobody cares about new functionality or history, so it absolutely does not matter what one can or cannot see in changelogs.
You are wrong if you believe that you can edit somebody else's ticket without asking for their opinion first. This is not a software development issue, this is called common sense.
Now, as you can see that you do not bring anything to the work going on here, let's stop this. Or continue by email/sagedevel if you like, but let's not polute this trac ticket.
The same "smart humans" you mentionned like to read the trac tickets message by message when they are done with reading the changelog, and we are just wasting their valuable time.
Nathann
comment:31 in reply to: ↑ 30 Changed 6 years ago by
Or perhaps I am wrong all the way from the start and nobody cares about new functionality or history, so it absolutely does not matter what one can or cannot see in changelogs.
You are wrong if you believe that you can edit somebody else's ticket without asking for their opinion first. This is not a software development issue, this is called common sense.
To be far, Nathann, I think the intent was just to clarify the intent of the ticket to those who might not know the whole history of the 'useless classes'. This seems pretty reasonable, since there are hundreds (if not thousands) of people who browse Trac. Humor is a pretty slippery thing  all about context :)
As to ownership, I would probably disagree that anyone "owns" any given ticket, any more than William "owns" Sage, but rather that there is significant moral authority vested in knowing the particular code and situation very well. So people shouldn't change titles without reason, but if there is a helpful title change (as this one was clearly intending to be) we can agree it's in the common good. Naturally, determining how the Sage community governs itself on this issue is for sagedevel  see also Steven Weber's "The Success of Open Source" for background reading on governance in opensource communities. Should really be required reading for anyone who spends more than a few hours a week on Trac :)
comment:32 Changed 6 years ago by
 Owner changed from (none) to ncohen
(I don't even know what this field is for, but I need to put my name there for political reasons)
comment:33 Changed 6 years ago by
Oh, by the way I answered the sagedevel thread : https://groups.google.com/forum/#!topic/sagedevel/do7EAtPq4Kg
Nathann
comment:34 Changed 6 years ago by
 Summary changed from Useless new design classes and a replacement for _check_pbd to New empty design classes for a better user interface and new is_group_divisible_design Cython function
comment:35 Changed 6 years ago by
 Commit changed from 494d82657000af8d4361a630035227c6aab6f5de to e8251fb1c1f22a1b7c6a15bd8be9686cdd5f54e6
Branch pushed to git repo; I updated commit sha1. New commits:
e8251fb  trac #16598: Merged with 6.3.beta6

comment:36 followup: ↓ 37 Changed 6 years ago by
 Status changed from needs_review to needs_work
Hi Nathann,
1) There is a problem with your relabel
method
sage: I = IncidenceStructure(3, [[0,1,2],[0,1]]) sage: I.relabel({0:'a',1:'b',2:'c'}) sage: I._point_to_index {0: 0, 1: 1, 2: 2}
but should be {'a':0, 'b': 1, 'c': 2}
.
2) For the string representation of divisible design, would not you prefer Group Divisible Design on 40 points of type 10^4
instead of the current Group Divisible Design on 40 points and type 10^4
3) This is very bad
round(sqrt(len(blocks)))
(i.e. it involves the symbolic ring). You would prefer using the method sqrtrem
of Sage integers
sage: i,j = 103.sqrtrem() sage: 103 == i**2 + j True
4) In is_group_divisible_design
the parts # Check that the groups belong to [0,...,n1]
and # Check that the groups belong to [0,...,n1]
are very complicated. It is not clear to me that it would be the fastest option (did you do some timings?). Would you mind having a more Pythonic version:
# Check that the groups belong to [0,...,n1] if any(i < 0 or i >= n for g in groups for i in g): # bad groups ... if any(i < 0 or i >= n for b in blocks for i in b): # bad blocks ...
That's all for now.
Vincent
comment:37 in reply to: ↑ 36 ; followup: ↓ 42 Changed 6 years ago by
 Status changed from needs_work to needs_review
YOoooooooo !
1) There is a problem with your
relabel
method
Gloops. Sorry 'bout that :/
2) For the string representation of divisible design, would not you prefer
Group Divisible Design on 40 points of type 10^4
instead of the currentGroup Divisible Design on 40 points and type 10^4
I don't care. I changed it,
3) This is very bad
round(sqrt(len(blocks)))(i.e. it involves the symbolic ring).
Is it bad to involve the symbolic ring ? O_o
I imported sqrt from Python instead.
4) In
is_group_divisible_design
the parts# Check that the groups belong to [0,...,n1]
and# Check that the groups belong to [0,...,n1]
are very complicated. It is not clear to me that it would be the fastest option (did you do some timings?). Would you mind having a more Pythonic version:# Check that the groups belong to [0,...,n1] if any(i < 0 or i >= n for g in groups for i in g): # bad groups ... if any(i < 0 or i >= n for b in blocks for i in b): # bad blocks ...
This code raises an exception when i cannot be turned into an integer. I simplified it a bit, though.
Nathann
comment:38 Changed 6 years ago by
 Commit changed from e8251fb1c1f22a1b7c6a15bd8be9686cdd5f54e6 to 8a44b83e01080869d82e96c265d73761e9a779b6
Branch pushed to git repo; I updated commit sha1. New commits:
8a44b83  trac #16598: Review

comment:39 followup: ↓ 40 Changed 6 years ago by
Hi,
Could we move _relabel_bibd
as a method of BalancedIncompleteBlockDesign
?
(not very important) Why did you put the classes PairwiseBalancedDesign
and BalancedIncompleteBlockDesign
in the file bibd.py
and not in incidence_structure.py
? I am not sure about what is the best organization should be, but right now it is not clear at all where to look to find something.
Vincent
comment:40 in reply to: ↑ 39 Changed 6 years ago by
Could we move
_relabel_bibd
as a method ofBalancedIncompleteBlockDesign
?
Hmmmm.. Well, it woud mean that we have to build BIBD objects during the recursive constructions too... I'd prefer to keep it this way if you don't mind :/
(not very important) Why did you put the classes
PairwiseBalancedDesign
andBalancedIncompleteBlockDesign
in the filebibd.py
and not inincidence_structure.py
? I am not sure about what is the best organization should be, but right now it is not clear at all where to look to find something.
Well, that BalancedIncompleteBlockDesign
is defined is bibd.py
is hardly a surprise. I feel that PBD are very close to BIBD but indeed, if GDD are in IncidenceStructure then indeed it can be moved there. What do you think ?
Nathann
comment:41 Changed 6 years ago by
 Commit changed from 8a44b83e01080869d82e96c265d73761e9a779b6 to ea8b144f5dd28d40102e5ccb06edab716951867a
Branch pushed to git repo; I updated commit sha1. New commits:
ea8b144  trac #16598: Code simplification

comment:42 in reply to: ↑ 37 ; followup: ↓ 44 Changed 6 years ago by
Replying to ncohen:
YOoooooooo !
1) There is a problem with your
relabel
methodGloops. Sorry 'bout that
:/
There is still a subtle issue. When we ask for a relabeling we do not necessarily want to relabel the points. More precisely, if the ground set is {0,1,2,3} and I want to relabel {0:1, 1:3, 2:1, 3:0} I would like the ground set to be fixed and the blocks to be changed... what do you think?
3) This is very bad
round(sqrt(len(blocks)))(i.e. it involves the symbolic ring).
Is it bad to involve the symbolic ring ?
O_o
Yes: SR is very slow and unreliable (though for sqrt of integers it should be ok).
Vincent
comment:43 Changed 6 years ago by
updated version at u/vdelecroix/16598 (above 6.3.rc1 and #16766)
Vincent
comment:44 in reply to: ↑ 42 ; followup: ↓ 47 Changed 6 years ago by
Yoooooooo !!
There is still a subtle issue. When we ask for a relabeling we do not necessarily want to relabel the points. More precisely, if the ground set is {0,1,2,3} and I want to relabel {0:1, 1:3, 2:1, 3:0} I would like the ground set to be fixed and the blocks to be changed... what do you think?
I don't understand. You are just relabeling with the same ground sets, what's the problem with that ? And how does that badly affect the blocks ? Some will change, some will not, what's the problem ?
Yes: SR is very slow and unreliable (though for sqrt of integers it should be ok).
"Bôf". If it does not work it's a bug report, and if Sage cannot compute square roots safely we should fork it quick :P
Nathann
comment:45 Changed 6 years ago by
 Branch changed from u/ncohen/16598 to u/vdelecroix/16598
 Commit changed from ea8b144f5dd28d40102e5ccb06edab716951867a to 1a9ae44eb163fd218e592c47ecb49a87cb7741d2
New commits:
081aa8b  trac #16598: merge with 6.3.rc1

49ae09d  trac #16766: Improve the doc of combinat/designs/

d626290  trac #16766: we don t want designs.deprecated_function_alias

8f9ee77  trac #16766: Broken doctests

47cebb3  trac #16766: Broken doctest

6cda4b6  trac #16766: Git 101: How to create a conflict with 10 others patches in needs_review

06e330b  trac #16766: form > from

1a9ae44  trac #16598: merge #16766 (documentation update)

comment:46 Changed 6 years ago by
 Dependencies changed from #16553 to #16553, #16766
comment:47 in reply to: ↑ 44 ; followup: ↓ 48 Changed 6 years ago by
Replying to ncohen:
Yoooooooo !!
There is still a subtle issue. When we ask for a relabeling we do not necessarily want to relabel the points. More precisely, if the ground set is {0,1,2,3} and I want to relabel {0:1, 1:3, 2:1, 3:0} I would like the ground set to be fixed and the blocks to be changed... what do you think?
I don't understand. You are just relabeling with the same ground sets, what's the problem with that ? And how does that badly affect the blocks ? Some will change, some will not, what's the problem ?
sage: I = IncidenceStructure(4,[[0,1,2],[1,3]]) sage: I.ground_set() [0, 1, 2, 3] sage: I.relabel({0:3,1:2,2:1,3:0}) sage: I.ground_set() [3, 2, 1, 0]
the ground set has changed...
Vincent
comment:48 in reply to: ↑ 47 Changed 6 years ago by
Yooooo !
sage: I = IncidenceStructure(4,[[0,1,2],[1,3]]) sage: I.ground_set() [0, 1, 2, 3] sage: I.relabel({0:3,1:2,2:1,3:0}) sage: I.ground_set() [3, 2, 1, 0]the ground set has changed...
Well, as a list it has, not as a set...
I think that being smart here only raises new problems. Relabelling all blocks takes much more time and memory than relabelling the points only. We could hide that by returning a sorted list of points, but well...
Nathann
comment:49 Changed 6 years ago by
 Status changed from needs_review to positive_review
Hello,
All right, we will see in the future.
Vincent
comment:50 Changed 6 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:52 Changed 6 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_work to positive_review
comment:53 Changed 6 years ago by
 Branch changed from u/vdelecroix/16598 to public/16598
 Commit changed from 1a9ae44eb163fd218e592c47ecb49a87cb7741d2 to 5f1fb98b5a72459b220b87f1c902bda62e4a166a
New commits:
5f1fb98  trac #16598: Merged with 6.3

comment:54 Changed 6 years ago by
 Branch changed from public/16598 to 5f1fb98b5a72459b220b87f1c902bda62e4a166a
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #16553: Clean IncidenceStructure