Complexes 26 06

[mikestillman]

We (@ggsmith and me) include here changes that fix a number of issues. We fixed the lazy resolution issue, but this might conflict with some new code that @mahrud is writing. I am marking this draft, although I hope we can get this all figured out in time for this release!

I will put other things that need to be fixed here, or in issue #3778.

[mahrud]

I'm still working on the Varieties PR, but seems like a test is failing here?

[mahrud]

???

[mahrud]

After further testing, this change breaks a significant portion of code I use for my research and packages I've shared with collaborators.

[mikestillman]

After further testing, this change breaks a significant portion of code I use for my research and packages I've shared with collaborators.

I think that using C.dd.maps directly can't possibly be compatible with lazy evaluation of maps (even if we move "Function" key out of that hash table.

Perhaps we can come up with a specific form that you want that would not use that, but could be used with it (i.e. a function in Complexes that does what you want without exposing C.dd.maps). In any case, we should be able to easily create a function that does what you want, that is as efficient, or at least close to it.

I'm still surprised that the list version is so much slower than the hash table version. Perhaps you sent me an example, but if so, I lost it or it got buried...

@mahrud @ggsmith

[mahrud]

I think that using C.dd.maps directly can't possibly be compatible with lazy evaluation of maps (even if we move "Function" key out of that hash table.

What other proposals have you considered? Adding a function key alongside integers seems like a clear anti-pattern, but maybe simpler, possibly more naive solutions didn't work for reasons I'm not aware of?

I'm still surprised that the list version is so much slower than the hash table version. Perhaps you sent me an example, but if so, I lost it or it got buried...

I mean, try it with literally any example you want. Here's a 9x slowdown:

i1 : R = kk[10];

i2 : C = koszulComplex vars R;

i3 : (lo,hi) = concentration C;

i4 : benchmark "complex C.dd.map"

o4 = .0001688285676264596

o4 : RR (of precision 53)

i5 : benchmark "complex(for i from lo+1 to hi list C.dd_i)"

o5 = .001522935406657019

o5 : RR (of precision 53)

i6 : oo/ooo

o6 = 9.020602544153419

o6 : RR (of precision 53)

Perhaps we can come up with a specific form that you want that would not use that, but could be used with it (i.e. a function in Complexes that does what you want without exposing C.dd.maps). In any case, we should be able to easily create a function that does what you want, that is as efficient, or at least close to it.

I doubt there's anything involving lists or functions that is going to beat direct access to a hash table.

i7 : benchmark "complex(for i from lo+1 to hi list C.dd.map#i)"

o7 = .001586263682279463

o7 : RR (of precision 53)

i8 : benchmark "complex(for i from lo+1 to hi list dd^C_i)"

o8 = .001606094099846388

o8 : RR (of precision 53)

[mikestillman]

@ggsmith @d-torrance @mahrud Where do we stand with this? I think we need to do the following

1. Move the Function key out of map. (see 3 below).
2. Make sure that the res14.m2 bug was actually fixed.
3. @mahrud Is there a stopgap measure we can do that won't mess you up too badly?
   My suggestion: take Function up one step (as you say, that is probably a better idiom anyway (@ggsmith are you fine with that?). Maybe: have a function which makes a lazy complex non-lazy, in that it fills in all non-zero maps in the concentration.

[mikestillman]

We also need to remove the hack version map(Complex, Complex, Function, ZZ), what should it be called?

[mahrud]

@mikestillman what other proposals have you considered? e.g since computing C.dd.source and C.dd.target are fast, what if we made just C.dd.map mutable instead of all of C.dd?

Maybe: have a function which makes a lazy complex non-lazy, in that it fills in all non-zero maps in the concentration.

complete?