Macaulay2 / M2

The primary source code repository for Macaulay2, a system for computing in commutative algebra, algebraic geometry and related fields.
https://macaulay2.com
334 stars 228 forks source link

directSum of MutableHashtable s (including ChainComplex'es) should use the cache #606

Open mikestillman opened 7 years ago

mikestillman commented 7 years ago

(from a discussion with Greg Smith)

The following code, when run, shows that the stashed value (directSum,C,C) is going directly into the ChainComplex object, and not into its cache table. But this info really should be in the cache table, and for our new Chain Complex code, it is necessary.

  S = ZZ/101[a..d]
  C = res coker vars S
  D = C ++ C
  keys C -- (directSum,C,C) this should be in the cache table, not in the mutable hash table itself.
  keys C.cache

The following changes to directSum (in m2/matrix.m2, around line 289) would fix this issue.

single = v -> (
     if not same v 
     then error "incompatible objects in direct sum";
     v#0)
directSum Sequence := args -> (
    if #args === 0 then error "expected more than 0 arguments";
    y := youngest args;
    key := (directSum, args);
    if y =!= null and y.cache#?key then y.cache#key else (
        type := single apply(args, class);
        meth := lookup(symbol directSum, type);
        if meth === null then error "no method for direct sum";
        S := meth args;
        if y =!= null then y.cache#key = S;
        S))

Note that stashing is not occuring for modules, since youngest only applies to mutable hash tables, and strangely doesn't apply to cache tables either, even though a cache table is a mutable hash table.

A more ambitious change would be to have 'youngest' work with cache tables, and change the code above to look at the youngest cache table of the given argument.

DanGrayson commented 7 years ago

A cache table is a special kind of mutable hash table whose hash code is 0. The age of a mutable hash table is defined to be its hash code -- that's why youngest doesn't work for them.

Your suggested change looks okay to me. We can hope that every type of object for which direct sums are defined also already has a cache table, and arrange for it if tests show that not to be the case.

mikestillman commented 7 years ago

@DanGrayson: it turns out that, (at least sometimes), if M is a Module, then M.cache exists (as it should), but also M.cache.cache does. Do you recall if there was a rationale for that, or should it be considered a bug? Interestingly, M.cache.cache is a MutableHashTable, not a CacheTable.

  S = ZZ/101[a..d]
  M = S^2
  peek M.cache
  peek M.cache.cache
mikestillman commented 7 years ago

In modules.m2, line 130, it explains that the reason the double cache is there is precisely so we can use 'youngest', to stash things like direct sums, Hom, tensors, so I guess it isn't a bug.

DanGrayson commented 7 years ago

I forgot all about that!

mikestillman commented 2 years ago

@DanGrayson, @ggsmith Is this still a problem? I can't reproduce the error.

DanGrayson commented 2 years ago

I'm not sure what the original issue was, but the value of C++C is no longer cached inside of C -- not in C and not in C.cache. That doesn't qualify as a bug.

DanGrayson commented 2 years ago

The caching ended in version 1.17.