On Mon, Nov 3, 2008 at 1:18 PM, Andreas Klöckner <lists@informa.tiker.net> wrote:
> I am used to the Matplotlib/MATLAB style, in which horizontal raster lines
> on the image are rows in the matrix (corresponds to the same order you
> write out a matrix).  So horizontally contiguous pixels are contiguous in
> memory, the opposite of what you describe.
>
> I think it therefore makes sense to add an extra parameter (as you suggest)
> which defaults to your current behavior but allows users like me to specify
> the transposed format.  Here are my code changes in driver.py:
>
> def matrix_to_array(matrix, matrixOrder):
> ...[snip]
> and
>
> def matrix_to_texref(matrix, texref, order="F"):
>     bind_array_to_texref(matrix_to_array(matrix, order), texref)
>
> There may also be other functions like make_multichannel_2d_array that may
> require similar modification.

I've applied this, with two changes: I've renamed matrixOrder to just order,
and I haven't defaulted it to "F". This breaks existing code, but in an
obvious way. Since it's non-obvious, people should give this some thought, and
the extra parameter reminds them that there is something to be thought about.
Protest?

Fine with me.  I am using the non-default parameter value anyway.  I think it is good for people to think about whether the matrix is row- or column- major order.
 
> Your DiskDict didn't work for me.

What was the error message? If something doesn't work out, I wrote to fail
over to a memory dict, apparently that bit's broken, too. Since I use that
code elsewhere, too, I'd be happy to know what broke.

It did notify my that since I didn't have Sqlite installed, it would be using the memory-based version (which obviously wouldn't persist between sessions, so that isn't great).  Here is the error message I was getting:

[from driver.py]

    398     return join(expanduser('~', ".pycuda-compiler-cache.diskdict")
    399
--> 400 _compile_cache = DiskDict(_get_cache_filename())
    401
    402

ValueError: dictionary update sequence element #0 has length 1; 2 is required
 


> I changed my disk-based cache code to make it more efficient.  Instead of
> creating temp files, if a cache directory is specified, it directly
> compiles into that directory.  Here is the new code (at the bottom of
> driver.py):
> [snip]

I've merged this, too. Couple changes, though:

- cache_dir defaults to ~/.pycuda-compiler-cache. Having to compute that in
every user program is not so nice. Having it overridable is nice, though.
Also, if you pass cache_dir=False, then caching is turned off.

- I took out the 'added efficiency' because that means `keep` and caching
aren't orthogonal any more. And in the presence of nvcc's garish slowness, an
extra temp directory is pretty much irrelevant.

OK, 

One (hypothetical) concern:
- Should we add code to catch MD5 collisions?

Shouldn't be an issue.  MD5 is a 128-bit hash especially designed to make sure that collisions will never happen.
 
Two nits for the future:
- Please stick to PEP 8 [1] when naming variables.
- Please send patches as attached unified diffs, rather than inlined source
blobs.

Noted :-)
 
Thanks for your input!

No problem.
 

The results of this are in git now, and I'll release them as 0.91rc2. Please
test.

On my to-do list...
 


Andreas

[1] http://www.python.org/dev/peps/pep-0008/