Skip to content

Reorder methods after new implications are added #2521

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.

Sign up for GitHub

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

Closed
stevelinton wants to merge 12 commits into from
Closed

Reorder methods after new implications are added #2521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
be8920b
Record rank adjustment in METHODS_OPERATION
stevelinton Jun 6, 2018
7812f31
Add RECALCULATE_ALL_METHOD_RANKS function which does what it says.
stevelinton Jun 6, 2018
a3cabba
Increase size of HIDDEN_IMPS_CACHE and WITH_IMPS_CACHE to speed up RE…
stevelinton Jun 13, 2018
953bc86
Make sure RANK_FILTERS is always set promptly, then remove the test f…
stevelinton Jun 13, 2018
5c4a76c
Automatically reorder methods after library or package loading, or In…
stevelinton Jun 13, 2018
d51f79e
Add test
stevelinton Jun 14, 2018
1d1279d
Documentation
stevelinton Jun 15, 2018
97cecad
HPCGAP support
stevelinton Jun 15, 2018
533c1b6
Handle Constructors
stevelinton Jun 19, 2018
d03cd97
Use COPY_LIST_ENTRIES in method installation to avoid making an inter…
stevelinton Jun 19, 2018
e29f5b9
Support passing a function of no arguments as the rank adjustment to …
stevelinton Jun 28, 2018
28d3d3f
Reset reordering status on quit from break loop.
stevelinton Jun 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/ref/methsel.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ one can install also <E>immediate methods</E>.
<Heading>Logical Implications</Heading>

<#Include Label="InstallTrueMethod">
<#Include Label="MethodReordering">
Copy link
Member

@fingolfin fingolfin Jun 15, 2018

Choose a reason for hiding this comment

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

Do we really want to make this publicly documented? Why?


</Section>

Expand Down
3 changes: 3 additions & 0 deletions lib/error.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ BIND_GLOBAL( "OnQuit", # care to ensure it always has a definition. - GG
until IsEmpty(OptionsStack);
Info(InfoWarning,1,"Options stack has been reset");
fi;
if IsBound(ResetMethodReordering) and IsFunction(ResetMethodReordering) then
ResetMethodReordering();
fi;
end);


Expand Down
79 changes: 73 additions & 6 deletions lib/filter.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,62 @@ BIND_GLOBAL( "InstallTrueMethodNewFilter", function ( tofilt, from )
end );


#############################################################################
##
#F SuspendMethodReordering( )
#F ResumeMethodReordering( )
#F ResetMethodReordering( )
##
## <#GAPDoc Label="MethodReordering">
## <ManSection>
## <Func Name="SuspendMethodReordering" Arg=""/>
## <Func Name="ResumeMethodReordering" Arg=""/>
## <Func Name="ResetMethodReordering" Arg=""/>
##
## <Description>
## These functions control whether the method reordering process
## described in <Ref Func="InstallTrueMethod"/> is invoked or not. Since this
## process can be comparatively time-consuming, it is usually suspended when
## a lot of implications are due to be installed, for instance when loading
## the library, or a package. This is done by calling <C>SuspendMethodReordering()</C>
## once the installations are done, <C>ResumeMethodReordering()</C> should be called.
## These pairs of calls can be nested. When the outermost pair is complete, method
## reordering takes place and is enabled in <C>InstallTrueMethod</C> thereafter.
##
## <C>ResetMethodReordering()</C> effectively exits all nested suspensions, resuming
## reordering immediately. This function is mainly provided for error recovery and
## similar purposes and is called on quitting from a break loop.
## </Description>
## </ManSection>
## <#/GAPDoc>

#
# This function will be defined in oper.g
#
RECALCULATE_ALL_METHOD_RANKS := fail;

REORDER_METHODS_SUSPENSION_LEVEL := 1;

BIND_GLOBAL( "SuspendMethodReordering", function()
REORDER_METHODS_SUSPENSION_LEVEL := REORDER_METHODS_SUSPENSION_LEVEL + 1;
end);


BIND_GLOBAL( "ResumeMethodReordering", function()
if REORDER_METHODS_SUSPENSION_LEVEL > 0 then
REORDER_METHODS_SUSPENSION_LEVEL := REORDER_METHODS_SUSPENSION_LEVEL - 1;
fi;
if REORDER_METHODS_SUSPENSION_LEVEL <= 0 then
RECALCULATE_ALL_METHOD_RANKS();
fi;
end);

BIND_GLOBAL( "ResetMethodReordering", function()
REORDER_METHODS_SUSPENSION_LEVEL := 0;
RECALCULATE_ALL_METHOD_RANKS();
end);


#############################################################################
##
#F InstallTrueMethod( <newfil>, <filt> )
Expand Down Expand Up @@ -209,18 +265,33 @@ end );
## This means that after the above implication has been installed,
## one can rely on the fact that every object in the filter
## <C>IsGroup and IsCyclic</C> will also be in the filter
## <Ref Func="IsCommutative"/>.
## <Ref Func="IsCommutative"/>.<P/>
##
## Adding logical implications can change the rank of filters
## (see <Ref Func="RankFilter"/>) and consequently the rank, and so choice of
## methods for operations (see <Ref Sect="Applicable Methods and Method Selection"/>).
## By default <C>InstallTrueMethod</C> adjusts the method selection data structures
## to take care of this, but this process can be time-consuming, so functions
## <Ref Func="SuspendMethodReordering"/> and <Ref Func="ResumeMethodReordering"/>
## are provided to allow control of this process.
Copy link
Member

@fingolfin fingolfin Jun 15, 2018

Choose a reason for hiding this comment

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

I think it's good to document this somewhere. But do we really want to document it for the public? I.e. perhaps move this out of the GAPDoc comment; or put it inside an XML comment?

## </Description>
## </ManSection>
## <#/GAPDoc>
##

BIND_GLOBAL( "InstallTrueMethod", function ( tofilt, from )

InstallTrueMethodNewFilter( tofilt, from );

# clear the caches because we do not know if filter <from> is new
CLEAR_HIDDEN_IMP_CACHE( from );
CLEAR_IMP_CACHE();

# maybe rerank methods to take account of new implication
if REORDER_METHODS_SUSPENSION_LEVEL = 0 then
RECALCULATE_ALL_METHOD_RANKS();
fi;

end );


Expand Down Expand Up @@ -424,11 +495,7 @@ BIND_GLOBAL( "RankFilter", function( filter )
all := WITH_IMPS_FLAGS(flags);
fi;
for i in TRUES_FLAGS(all) do
if IsBound(RANK_FILTERS[i]) then
rank := rank + RANK_FILTERS[i];
else
rank := rank + 1;
fi;
od;
return rank;
end );
Expand Down
5 changes: 5 additions & 0 deletions lib/init.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,9 @@ fi;
##
## Read init files, run a shell, and do exit-time processing.
##

ResumeMethodReordering();

InstallAndCallPostRestore( function()
local i, status;
for i in [1..Length(GAPInfo.InitFiles)] do
Expand All @@ -1037,6 +1040,8 @@ InstallAndCallPostRestore( function()
od;
end );



if IsLIBGAP then
# GAP is used as a library, do not start an interactive session
elif IsHPCGAP and THREAD_UI() then
Expand Down
170 changes: 150 additions & 20 deletions lib/oper.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,7 @@ BIND_GLOBAL( "DeclareAttributeKernel", function ( name, filter, getter )
FILTERS[ FLAG2_FILTER( tester ) ] := tester;
IMM_FLAGS:= AND_FLAGS( IMM_FLAGS, FLAGS_FILTER( tester ) );
INFO_FILTERS[ FLAG2_FILTER( tester ) ] := 5;
RANK_FILTERS[ FLAG2_FILTER( tester ) ] := 1;
od;

# clear the cache because <filter> is something old
Expand All @@ -1107,10 +1108,6 @@ BIND_GLOBAL( "DeclareAttributeKernel", function ( name, filter, getter )
# run the attribute functions
RUN_ATTR_FUNCS( filter, getter, setter, tester, false );

# store the ranks
atomic FILTER_REGION do
RANK_FILTERS[ FLAG2_FILTER( tester ) ] := 1;
od;

# and make the remaining assignments
nname:= "Set"; APPEND_LIST_INTR( nname, name );
Expand Down Expand Up @@ -1198,6 +1195,8 @@ BIND_GLOBAL( "OPER_SetupAttribute", function(getter, flags, mutflag, filter, ran
# install the default functions
FILTERS[ FLAG2_FILTER( tester ) ] := tester;
IMM_FLAGS:= AND_FLAGS( IMM_FLAGS, FLAGS_FILTER( tester ) );
# store the rank
RANK_FILTERS[ FLAG2_FILTER( tester ) ] := rank;

# the <tester> is newly made, therefore the cache cannot contain a flag
# list involving <tester>
Expand All @@ -1209,8 +1208,6 @@ BIND_GLOBAL( "OPER_SetupAttribute", function(getter, flags, mutflag, filter, ran
# run the attribute functions
RUN_ATTR_FUNCS( filter, getter, setter, tester, mutflag );

# store the rank
RANK_FILTERS[ FLAG2_FILTER( tester ) ] := rank;


return;
Expand Down Expand Up @@ -1439,6 +1436,10 @@ BIND_GLOBAL( "DeclarePropertyKernel", function ( name, filter, getter )
INFO_FILTERS[ FLAG1_FILTER( getter ) ]:= 7;
INFO_FILTERS[ FLAG2_FILTER( getter ) ]:= 8;

# store the ranks
RANK_FILTERS[ FLAG1_FILTER( getter ) ] := 1;
RANK_FILTERS[ FLAG2_FILTER( getter ) ] := 1;

# clear the cache because <filter> is something old
if not GAPInfo.CommandLineOptions.N then
InstallHiddenTrueMethod( tester, getter );
Expand All @@ -1450,9 +1451,6 @@ BIND_GLOBAL( "DeclarePropertyKernel", function ( name, filter, getter )
# run the attribute functions
RUN_ATTR_FUNCS( filter, getter, setter, tester, false );

# store the ranks
RANK_FILTERS[ FLAG1_FILTER( getter ) ] := 1;
RANK_FILTERS[ FLAG2_FILTER( getter ) ] := 1;

# and make the remaining assignments
nname:= "Set"; APPEND_LIST_INTR( nname, name );
Expand Down Expand Up @@ -1513,8 +1511,14 @@ BIND_GLOBAL( "NewProperty", function ( arg )
FILTERS[ FLAG2_FILTER( getter ) ] := tester;
INFO_FILTERS[ FLAG1_FILTER( getter ) ] := 9;
INFO_FILTERS[ FLAG2_FILTER( getter ) ] := 10;
if LEN_LIST( arg ) = 3 and IS_INT( arg[3] ) then
RANK_FILTERS[ FLAG1_FILTER( getter ) ]:= arg[3];
else
RANK_FILTERS[ FLAG1_FILTER( getter ) ]:= 1;
fi;
RANK_FILTERS[ FLAG2_FILTER( tester ) ]:= 1;
od;

# the <tester> and <getter> are newly made, therefore the cache cannot
# contain a flag list involving <tester> or <getter>
if not GAPInfo.CommandLineOptions.N then
Expand All @@ -1526,15 +1530,6 @@ BIND_GLOBAL( "NewProperty", function ( arg )
# run the attribute functions
RUN_ATTR_FUNCS( filter, getter, setter, tester, false );

# store the rank
atomic FILTER_REGION do
if LEN_LIST( arg ) = 3 and IS_INT( arg[3] ) then
RANK_FILTERS[ FLAG1_FILTER( getter ) ]:= arg[3];
else
RANK_FILTERS[ FLAG1_FILTER( getter ) ]:= 1;
fi;
RANK_FILTERS[ FLAG2_FILTER( tester ) ]:= 1;
od;

# and return the getter
return getter;
Expand Down Expand Up @@ -1931,7 +1926,8 @@ end);

fi;

if BASE_SIZE_METHODS_OPER_ENTRY <> 5 then

if BASE_SIZE_METHODS_OPER_ENTRY <> 6 then
Error("MethodsOperation must be updated for new BASE_SIZE_METHODS_OPER_ENTRY");
fi;

Expand All @@ -1952,6 +1948,7 @@ BIND_GLOBAL("MethodsOperation", function(oper, nargs)
func := meths[i + nargs + 2],
rank := meths[i + nargs + 3],
info := meths[i + nargs + 4],
rankbase := meths[i + nargs + 6],
);
ADD_LIST(result, m);
if IsBound(meths[i + nargs + 5]) then
Expand All @@ -1961,6 +1958,139 @@ BIND_GLOBAL("MethodsOperation", function(oper, nargs)
return result;
end );

#############################################################################
##
#F RECALCULATE_ALL_METHOD_RANKS() . . reorder methods after new implications
##
## Installing new implications (including hidden implications) can change the
## rank of existing filters, and so of existing methods for operations.
##
## This function recalculates all such ranks and adjusts the method ordering
## where needed. If the ordering changes, the relevant caches are flushed.
##
## If PRINT_REORDERED_METHODS is true, it prints some diagnostics (this is a
## bit too low-level for Info).
##
##


#
# We had to install a placeholder for this in filter.g
#
Unbind(RECALCULATE_ALL_METHOD_RANKS);

PRINT_REORDERED_METHODS := false;

BIND_GLOBAL( "RECALCULATE_ALL_METHOD_RANKS", function()
local oper, n, changed, meths, nmethods, i, base, rank, j, req,
req2, k, l;

for oper in OPERATIONS do
for n in [0..6] do
changed := false;
meths := METHODS_OPERATION(oper, n);
nmethods := LENGTH(meths)/(BASE_SIZE_METHODS_OPER_ENTRY+n);
if IS_CONSTRUCTOR(oper) and n > 0 then
for i in [nmethods,nmethods-1..1] do
base := (i-1)*(BASE_SIZE_METHODS_OPER_ENTRY+n);
# data for this method is meths{[base+1..base+BASE_SIZE_METHODS_OPER_ENTRY + n]}
rank := meths[base+6+n];
if IS_FUNCTION(rank) then
rank := rank();
fi;
rank := rank - RankFilter(WITH_IMPS_FLAGS(meths[base+2]));
if rank <> meths[base+n+3] then
if IsHPCGAP and not changed then
meths := SHALLOW_COPY_OBJ(meths);
fi;
changed := true;
meths[base+n+3] := rank;
fi;
# compare to rank of succeding method
if i = nmethods or rank >= meths[base+BASE_SIZE_METHODS_OPER_ENTRY + 2*n + 3] then
continue;
fi;
k := i+2;
while k <= nmethods and meths[(k-1)*(BASE_SIZE_METHODS_OPER_ENTRY+n) + n + 3] < rank do
k := k+1;
od;
k := k-1;
if PRINT_REORDERED_METHODS then
Print("Constructor ",NAME_FUNC(oper), " ", n," args. Moving method ",i," (",
meths[base+n+4]," from ",meths[base+n+5][1],":", meths[base+n+5][2],
") to position ",k,"\n");
fi;
l := meths{[base+1..base+n+BASE_SIZE_METHODS_OPER_ENTRY]};
COPY_LIST_ENTRIES(meths, 1 + i*(n+BASE_SIZE_METHODS_OPER_ENTRY), 1,
meths, 1 +(i-1)*(n+BASE_SIZE_METHODS_OPER_ENTRY), 1,
(k-i)*(n+BASE_SIZE_METHODS_OPER_ENTRY));
meths{[1 + (k-1)*(n+BASE_SIZE_METHODS_OPER_ENTRY)..
k*(n+BASE_SIZE_METHODS_OPER_ENTRY)]} := l;
od;
else
for i in [1 ..nmethods] do
base := (i-1)*(BASE_SIZE_METHODS_OPER_ENTRY+n);
# data for this method is meths{[base+1..base+BASE_SIZE_METHODS_OPER_ENTRY + n]}
rank := meths[base+6+n];
if IS_FUNCTION(rank) then
rank := rank();
fi;


for j in [1..n] do
req := meths[base+1+j];
rank := rank+RankFilter(WITH_IMPS_FLAGS(req));
od;

if rank <> meths[base+n+3] then
if IsHPCGAP and not changed then
meths := SHALLOW_COPY_OBJ(meths);
fi;
changed := true;
meths[base+n+3] := rank;
fi;

# compare to rank of preceding method
if i = 1 or rank <= meths[base-BASE_SIZE_METHODS_OPER_ENTRY+3] then
continue;
fi;

k := i-2;
while k > 1 and meths[(k-1)*(BASE_SIZE_METHODS_OPER_ENTRY+n) + n + 3] < rank do
k := k-1;
od;
k := k+1;
if PRINT_REORDERED_METHODS then
Print(NAME_FUNC(oper), " ", n," args. Moving method ",i," (",
meths[base+n+4]," from ",meths[base+n+5][1],":", meths[base+n+5][2],
") to position ",k,"\n");
fi;
l := meths{[base+1..base+n+BASE_SIZE_METHODS_OPER_ENTRY]};
COPY_LIST_ENTRIES(meths, 1 + (k-1)*(n+BASE_SIZE_METHODS_OPER_ENTRY), 1,
meths, 1 + k*(n+BASE_SIZE_METHODS_OPER_ENTRY), 1,
(i-k)*(n+BASE_SIZE_METHODS_OPER_ENTRY));
meths{[1 + (k-1)*(n+BASE_SIZE_METHODS_OPER_ENTRY)..
k*(n+BASE_SIZE_METHODS_OPER_ENTRY)]} := l;
od;
fi;
if changed then
if IsHPCGAP then
SET_METHODS_OPERATION(oper,n,MakeReadOnlySingleObj(meths));
else
CHANGED_METHODS_OPERATION(oper,n);
fi;
fi;
od;
od;
end );








#############################################################################
##
#E
Loading