Merge pull request #696 from zickgraf/caches

Fix bug in caching mechanism and add Object/MorphismConstructor to non-cached operations
homalg-project · Jul 31, 2021 · 74b865c · 74b865c
2 parents a57615f + dcc0ec8
commit 74b865c
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 35 deletions.
diff --git a/CAP/PackageInfo.g b/CAP/PackageInfo.g
@@ -21,7 +21,7 @@ Version := Maximum( [
   ## this line prevents merge conflicts
   "2020.04-16", ## Sepp's version
   ## this line prevents merge conflicts
-  "2021.07-01", ## Fabian's version
+  "2021.07-02", ## Fabian's version
   ## this line prevents merge conflicts
   "2021.05-04", ## Kamal's version
 ] ),

diff --git a/CAP/gap/CAP.gi b/CAP/gap/CAP.gi
@@ -27,9 +27,40 @@ InstallTrueMethod( IsPreAbelianCategory, IsAbelianCategory );
 
 ##
 InstallValue( CAP_INTERNAL,
-              rec(
-                   name_counter := 0,
-                   default_cache_type := "weak",
+                rec(
+                    name_counter := 0,
+                    default_cache_type := "weak",
+                    operation_names_with_cache_disabled_by_default := [
+                        # the following operations are needed for comparison in caches
+                        "IsEqualForObjects",
+                        "IsEqualForMorphisms",
+                        "IsEqualForMorphismsOnMor",
+                        "IsEqualForCacheForObjects",
+                        "IsEqualForCacheForMorphisms",
+                        # it is unclear how `IsEqualForCacheForObjects` and `IsEqualForCacheForMorphisms`
+                        # would behave on non-well-defined objects/morphisms, so exclude `IsWellDefined*`
+                        "IsWellDefinedForObjects",
+                        "IsWellDefinedForMorphisms",
+                        "IsWellDefinedForTwoCells",
+                        # do not cache operations returning random data
+                        "RandomObjectByInteger",
+                        "RandomMorphismByInteger",
+                        "RandomMorphismWithFixedSourceByInteger",
+                        "RandomMorphismWithFixedRangeByInteger",
+                        "RandomMorphismWithFixedSourceAndRangeByInteger",
+                        "RandomObjectByList",
+                        "RandomMorphismByList",
+                        "RandomMorphismWithFixedSourceByList",
+                        "RandomMorphismWithFixedRangeByList",
+                        "RandomMorphismWithFixedSourceAndRangeByList",
+                        # by default, do not cache constructors and object/morphism data
+                        # because in general these operations are cheap,
+                        # so caching would not improve the performance
+                        "ObjectConstructor",
+                        "ObjectDatum",
+                        "MorphismConstructor",
+                        "MorphismDatum",
+                    ],
               )
 );
 
@@ -80,7 +111,7 @@ InstallGlobalFunction( GET_METHOD_CACHE,
         cache := CreateWeakCachingObject( number );
     elif cache_type = "crisp" then
         cache := CreateCrispCachingObject( number );
-    elif cache_type = "none" or cache_type = "never" then
+    elif cache_type = "none" then
         cache := CreateCrispCachingObject( number );
         DeactivateCachingObject( cache );
     else
@@ -184,7 +215,7 @@ end );
 InstallGlobalFunction( "CREATE_CAP_CATEGORY_OBJECT",
 
   function( obj_rec, attr_list )
-    local i, flatted_attribute_list, obj;
+    local i, flatted_attribute_list, obj, operation_name;
 
     for i in [ 1 .. Length( attr_list ) ] do
 
@@ -215,30 +246,13 @@ InstallGlobalFunction( "CREATE_CAP_CATEGORY_OBJECT",
 
     obj!.derivations_weight_list := MakeOperationWeightList( obj, CAP_INTERNAL_DERIVATION_GRAPH );
 
-    obj!.caches := rec( IsEqualForObjects := "never",
-                        IsEqualForMorphisms := "never",
-                        IsEqualForMorphismsOnMor := "never",
-                        IsEqualForCacheForObjects := "never",
-                        IsEqualForCacheForMorphisms := "never",
-                        IsWellDefinedForObjects := "never",
-                        IsWellDefinedForMorphisms := "never",
-                        IsWellDefinedForTwoCells := "never",
-                        RandomObjectByInteger := "never",
-                        RandomMorphismByInteger := "never",
-                        RandomMorphismWithFixedSourceByInteger := "never",
-                        RandomMorphismWithFixedRangeByInteger := "never",
-                        RandomMorphismWithFixedSourceAndRangeByInteger := "never",
-                        RandomObjectByList := "never",
-                        RandomMorphismByList := "never",
-                        RandomMorphismWithFixedSourceByList := "never",
-                        RandomMorphismWithFixedRangeByList := "never",
-                        RandomMorphismWithFixedSourceAndRangeByList := "never",
-                        # object and morphism data must never be cashed
-                        # because `IsEqualForCache*` might involve them,
-                        # possibly leading to an infinite recursion
-                        ObjectDatum := "never",
-                        MorphismDatum := "never",
-                      );
+    obj!.caches := rec( );
+
+    for operation_name in CAP_INTERNAL.operation_names_with_cache_disabled_by_default do
+
+        obj!.caches.(operation_name) := "none";
+
+    od;
 
     obj!.redirects := rec( );
 
@@ -385,8 +399,7 @@ InstallMethod( SetCaching,
 
     fi;
 
-    if not IsBound( category!.caches.( function_name ) ) or
-        ( IsString( category!.caches.( function_name ) ) and category!.caches.( function_name ) <> "never" ) then
+    if not IsBound( category!.caches.( function_name ) ) or IsString( category!.caches.( function_name ) ) then
 
         category!.caches.( function_name ) := caching_info;
 
@@ -465,8 +478,8 @@ InstallGlobalFunction( SetCachingOfCategory,
 
     for current_name in RecNames( category!.caches ) do
 
-        if current_name in [ "IsEqualForMorphisms", "IsEqualForObjects", "IsEqualForMorphismsOnMor", "IsEqualForCacheForMorphisms", "IsEqualForCacheForObjects" ] then
-            continue; ## Those are needed for comparison in caches
+        if current_name in CAP_INTERNAL.operation_names_with_cache_disabled_by_default then
+            continue;
         fi;
 
         SetCaching( category, current_name, type );

diff --git a/CAP/gap/CategoryMorphisms.gd b/CAP/gap/CategoryMorphisms.gd
@@ -67,6 +67,8 @@ DeclareGlobalVariable( "PROPAGATION_LIST_FOR_EQUAL_MORPHISMS" );
 #! The arguments are two objects $S$ and $T$ in a category,
 #! and a morphism datum $a$ (type and semantics of the morphism datum depend on the category).
 #! The output is a morphism in $\mathrm{Hom}(S,T)$ defined by $a$.
+#! Note that by default this CAP operation is not cached. You can change this behaviour
+#! by calling `SetCachingToWeak( C, "MorphismConstructor" )` resp. `SetCachingToCrisp( C, "MorphismConstructor" )`.
 #! @Returns a morphism in $\mathrm{Hom}(S,T)$
 #! @Arguments S, a, T
 DeclareOperation( "MorphismConstructor",
@@ -95,6 +97,8 @@ DeclareOperation( "AddMorphismConstructor",
 #! The argument is a CAP category morphism <A>mor</A>.
 #! The output is a datum which can be used to construct <A>mor</A>, that is,
 #! `IsEqualForMorphisms( `<A>mor</A>`, MorphismConstructor( Source( `<A>mor</A>` ), MorphismDatum( `<A>mor</A>` ), Range( `<A>mor</A>` ) ) )`.
+#! Note that by default this CAP operation is not cached. You can change this behaviour
+#! by calling `SetCachingToWeak( C, "MorphismDatum" )` resp. `SetCachingToCrisp( C, "MorphismDatum" )`.
 #! @Returns depends on the category
 #! @Arguments mor
 DeclareAttribute( "MorphismDatum",

diff --git a/CAP/gap/CategoryObjects.gd b/CAP/gap/CategoryObjects.gd
@@ -399,6 +399,8 @@ DeclareGlobalFunction( "ObjectifyObjectForCAPWithAttributes" );
 #! The arguments are a category $C$ and an object datum $a$
 #! (type and semantics of the object datum depend on the category).
 #! The output is an object of $C$ defined by $a$.
+#! Note that by default this CAP operation is not cached. You can change this behaviour
+#! by calling `SetCachingToWeak( C, "ObjectConstructor" )` resp. `SetCachingToCrisp( C, "ObjectConstructor" )`.
 #! @Returns an object
 #! @Arguments C, a
 DeclareOperation( "ObjectConstructor",
@@ -427,6 +429,8 @@ DeclareOperation( "AddObjectConstructor",
 #! The argument is a CAP category object <A>obj</A>.
 #! The output is a datum which can be used to construct <A>obj</A>, that is,
 #! `IsEqualForObjects( `<A>obj</A>`, ObjectConstructor( CapCategory( `<A>obj</A>` ), ObjectDatum( `<A>obj</A>` ) ) )`.
+#! Note that by default this CAP operation is not cached. You can change this behaviour
+#! by calling `SetCachingToWeak( C, "ObjectDatum" )` resp. `SetCachingToCrisp( C, "ObjectDatum" )`.
 #! @Returns depends on the category
 #! @Arguments obj
 DeclareAttribute( "ObjectDatum",

diff --git a/FreydCategoriesForCAP/PackageInfo.g b/FreydCategoriesForCAP/PackageInfo.g
@@ -17,7 +17,7 @@ Version := Maximum( [
   ## this line prevents merge conflicts
   "2020.05-17", ## Mohamed's version
   ## this line prevents merge conflicts
-  "2021.07-02", ## Fabian's version
+  "2021.07-03", ## Fabian's version
   ## this line prevents merge conflicts
   "2020.04-18", ## Kamal's version
 ] ),

diff --git a/FreydCategoriesForCAP/gap/CategoryOfColumns.gi b/FreydCategoriesForCAP/gap/CategoryOfColumns.gi
@@ -37,6 +37,9 @@ InstallMethod( CategoryOfColumns,
         ),
     );
 
+    # this cache replaces the KeyDependentOperation caching when using ObjectConstructor directly instead of CategoryOfColumnsObject
+    SetCachingToWeak( category, "ObjectConstructor" );
+
     SetFilterObj( category, IsCategoryOfColumns );
 
     if HasHasInvariantBasisProperty( homalg_ring ) and HasInvariantBasisProperty( homalg_ring ) then

diff --git a/FreydCategoriesForCAP/gap/CategoryOfRows.gi b/FreydCategoriesForCAP/gap/CategoryOfRows.gi
@@ -37,6 +37,9 @@ InstallMethod( CategoryOfRows,
         ),
     );
 
+    # this cache replaces the KeyDependentOperation caching when using ObjectConstructor directly instead of CategoryOfRowsObject
+    SetCachingToWeak( category, "ObjectConstructor" );
+
     SetFilterObj( category, IsCategoryOfRows );
 
     if HasHasInvariantBasisProperty( homalg_ring ) and HasInvariantBasisProperty( homalg_ring ) then

diff --git a/LinearAlgebraForCAP/gap/LinearAlgebraForCAP.gi b/LinearAlgebraForCAP/gap/LinearAlgebraForCAP.gi
@@ -45,6 +45,9 @@ InstallGlobalFunction( MATRIX_CATEGORY,
         ),
     );
 
+    # this cache replaces the KeyDependentOperation caching when using ObjectConstructor directly instead of MatrixCategoryObject
+    SetCachingToWeak( category, "ObjectConstructor" );
+
     SetFilterObj( category, IsMatrixCategory );
 
     AddObjectRepresentation( category, IsVectorSpaceObject and HasIsProjective and IsProjective );