Mutable sumbuild aliases about knossos-ksc HOT 7 CLOSED

Ouch. Good spot. The logic we really want is this:

initialize result
for i in 0..n-1
   Mark the allocator
   BODY // may do lots of allocation, but ultimately leaves a result in bodyex
   Add bodyex to result -- effectively copying all its values
   Reset the heap
end

But we don't know how to init the result, because we don't know the sizes of things in general.

So, options:

• Initialize to zeros.
  Problem: we can't compute the size of everything easily. This is made harder by removing sizes, but was always going to be a problem for other types like lists and jagged arrays anyway.
• Implement DPS, then the return val's storage is supplied, so we just need to zero it.
• The current logic: compute one return value, mark the heap there, and use that.
  This has two issues: first, this bug, second, extra allocations in bodyex aren't freed.
• Use another heap temporarily, e.g. malloc.

I think the latter is best for now:

for i in 0..n-1
   Mark the allocator
   BODY // may do lots of allocation, but ultimately leaves a result in bodyex
   if i = 0, {
      result = malloc(bodyex size);  // Note: if we have the "is_zero" flag this may be harder.
   } 
   Add bodyex to result -- effectively copying all its values
   Reset the heap
   real_result = ks_alloc(size);
   copy result to real_result
   free(result);
end

from knossos-ksc.

Thanks, notice that if we still have the "is_zero" flag, then extra logic needs to go in the Haskell, which gets very messy. I would suggest moving sumbuild back to C++ with a lambda -- the compiler should deal with it without loss of speed.

from knossos-ksc.

[For the record, converting back from inline sumbuild to sumbuild which calls a lambda does not fix the bug. I suppose whether a lambda returns the s or not is irrelevant. The data of a vec will not be copied in any case.

To demonstrate this I just deleted the clause for Call of a "sumbuild".]

from knossos-ksc.

We need a recipe that is independent of the data type it is working on[1]. The code that handles sumbuild of a float also needs to handle sumbuild of a vec of of a tuple of a ... . In short, I don't think the above use-other-heap-with-malloc approach will work directly.

I suggest we define a templated copy function for each Knossos type (float, int, std::tuples of things, vecs of things) that creates a deep copy of any value. What do you think?

[1] We definitely need it if we stick to the inline sumbuild. We probably need it if we go back to the sumbuild-calls-a-lambda version. There's perhaps a chance that we can do something clever with templates but I think that probably addresses the problem in the wrong place.

from knossos-ksc.

[For the record, converting back from inline sumbuild to sumbuild which calls a lambda does not fix the bug. I suppose whether a lambda returns the s or not is irrelevant. The data of a vec will not be copied in any case.
To demonstrate this I just deleted the clause for Call of a "sumbuild".]

Agreed, this was just to move the logic out of the Haskell code in order to make it more maintainable in C++.

from knossos-ksc.

I suggest we define a templated copy function for each Knossos type (float, int, std::tuples of things, vecs of things) that creates a deep copy of any value. What do you think?

Agreed - only question is which heap it copies onto. I think it needs to be the malloc (C++ new/delete) heap, and then we need a copy_to_ks_heap version and a free_copy to run on the malloc heap.

And we should also ask how DPS will ultimately work, and whether it's as much effort to implement it now (I think it is too much, just noting).

from knossos-ksc.

Ah, I think we are proposing slightly different things. I am proposing a design that doesn't require a second heap but leaks the allocations in the first iteration of the loop. I think this will be a lot easier for me to implement (because my familiarity with custom allocators is pretty limited). But will it be too big a deal that it leaks?

for i in 0..n-1
   BODY // may do lots of allocation, but ultimately leaves a result in bodyex
   if i = 0 {
      result = deep_copy(bodyex);
   } 
   if i = 1 {
       mark the allocator;
   }
   if i != 0 {
       inplace_add(result, bodyex);
   }
end

reset the allocator

from knossos-ksc.