#674
  • Created on
    2011-05-30 19:58
  • Reported by
    nouiz
  • Status
    Fixed
  • Priority
    High (2)
  • Assigned to
    lamblin
  • Milestone
    0.6.2
  • Plan Level
    -
  • Due Date
    -

Attachments

Related Tickets

Add new subtask or bug

Review doc and all c code for the use of preallocated output.

Hi,

Currently in 2 cases we provide the output to use to the op(in the output_storage). This is not about inplace stuff. Most code have been written to check the shape of the output before reusing it. Most c code don't check the stride and suppose the output to be c contiguous.

In the current 2 cases, this is true, but we should review this and add this check if the code suppose the output to be c contiguous.

This is some place where it have been discussed:

UPDATE: Here are some things still left to do.
  • Do not preallocate all memory buffers at the same time, but one map at a time (only previous, then only c_contiguous, etc.).
  • More stride patterns, implement 'neg_stride' should be good.
  • Buffers with wrong shape (too small, too big, right overall size but wrong shape on some dimensions).
  • Copy only non-destroyed inputs (also for regular DebugMode).
  • New error or error messages that reflect more faithfully what the problem really is.
  • Check on memory buffers that were already provided in the initial storage_map.
User picture
  on 2011-06-07 06:37 *
By lamblin
Assigned to set to lamblin
Status changed from New to assigned
User picture
  on 2011-06-14 22:21 *
By lamblin
There is now a new flag, config.DebugMode.check_preallocated_output that tries various preallocated output storages.

I have yet to
- test more thoroughly the behaviour of this flag,
- run the whole test suite with it, to detect problems in the existing Ops.

Not so critical, but should be done at some point:
- test pre-allocated output for Sparse, maybe Scalar,
- test more patterns (negative strides, non-contiguous memory),

See also* 435.
User picture
  on 2011-06-17 01:15 *
By lamblin
Assigned to changed from lamblin to lamblin
Status changed from assigned to New
The whole test suite has been run with
- 'previous'
- 'c_contiguous'
- 'f_contiguous'

And all tests now pass.
User picture
  on 2011-06-22 04:16 *
By lamblin
The whole test suite has been run with
- 'previous'
- 'c_contiguous'
- 'f_contiguous'

And all tests now pass.
User picture
  on 2012-01-13 00:55 *
By nouiz
Assigned to changed from someone to -none-
User picture
  on 2012-01-13 01:09 *
By lamblin
Assigned to set to lamblin
Status changed from New to Accepted
User picture
  on 2012-04-12 20:30 *
By lamblin
The only things left to do are:
- New error or error messages that reflect more faithfully what the problem really is,
- Update the documentation.
User picture
  on 2012-04-14 03:07 *
By lamblin
Done in https://github.com/Theano/Theano/pull/604, I'll close the ticket when the PR is accepted.
User picture
  on 2013-04-09 00:41 *
By lamblin
Status changed from Accepted to Fixed