BinaryOperations can try to write into workspace of the wrong size

[Russell Taylor]

The following script will almost certainly crash Mantid (or give a debug assertion with a Windows debug build):

ws2d = Load('LOQ48127')
column = Integration(ws2d)
column = ws2d / column

The binary operation itself is valid; the problem is that it tries to put the result - which has the same dimensions as ws2d (a multi-bin 2d workspace) - into the existing single-bin column workspace instead of creating a new Workspace2D for the output. So internally it runs off the end of the vector.

Note that this issue is general to BinaryOperation, not just Divide, and that it cropped up in a less obvious place - when Divide was being run as a subalgorithm and the denominator workspace had ended up in the ADS called 'ChildAlgOutput' - which is the supposedly internal name given to an un-named output workspace of a child algorithm. (That's probably another bug.)

[Russell Taylor]

My preferred fix is to change line 231 of BinaryOperation.cpp from

            ( m_out == m_lhs && ( m_flipSides ) )  )

back to what it previously was (a long time ago)

             (m_out == m_rhs && ( m_lhs->size() > m_rhs->size() )) )

However, this makes one of the Plus unit tests fail. Specifically, something like this is currently forbidden, when I don't think it should be (although 'singlespec' should end up being a different workspace):

ws2d = Load('LOQ48127')
singlespec = SumSpectra(ws2d)
singlespec += ws2d

In fact, the Plus algorithm fails part way through, not at validation, so the numbers in the original 'singlespec' have already been changed (basically, it's the same bug as the column workspace case but we're saved from a crash because there's range checking when accessing a spectrum, but not when going to the bins of a single spectrum).

It's a bit disingenuous that if the above is made to work singlespec ends up being a different workspace, but at least the python variable is invalidated. I would say it is consistent with other things though. For example, it's akin to asking for Rebin(InputWorkspace=singlespec,OutputWorkspace=singlespec) - singlespec will end up being a different workspace with the same name.

[Russell Taylor]

trac was down when I pushed up:

Re #5876. Change how we decide whether to create output workspace.

Back to previous (before 6bde1e7 of Oct 2010) and, I believe, more correct way of deciding whether we need to create a new output workspace.

e780af7a7e2cfcf47c8164b3886f9e1ed6544bc0

and

Re #5876. Change a test that was failing after the previous commit.

It was testing that the Plus algorithm failed in a situation where I believe it should be succeeding (and it does now).

e5b031deb60f94b5f22b1a0abd82c5dbb3fa5495

[Russell Taylor]

In the case of the python example above, what I think needs to be done to fix this is to amend the test at line 89 of PythonInterface/mantid/api/_workspaceops.py to follow the 'else' branch in this case.

[Russell Taylor]

Re #5876. Always return output workspace of binary operation.

Always return the result workspace for compound binary operations as it's possible it could be a new workspace underneath (if a Workspace2D has changed size).

Changeset: 73b5b6d74bbe5ccc9b3693509f9973d74a584feb

[Russell Taylor]

To test: check that the scriptlets in the description and comment:2 run successfully and that the variable set in the final line points to the correct workspace. (N.B. The final workspace variable in python will be of type IMDWorkspace rather than MatrixWorkspace. This is the subject of ticket #6015.)

[Russell Taylor]

Re #5876. Change how we decide whether to create output workspace.

Back to previous (before 6bde1e7a of Oct 2010) and, I believe, more correct way of deciding whether we need to create a new output workspace.

Changeset: e780af7a7e2cfcf47c8164b3886f9e1ed6544bc0

[Russell Taylor]

Re #5876. Change a test that was failing after the previous commit.

It was testing that the Plus algorithm failed in a situation where I believe it should be succeeding (and it does now).

Changeset: e5b031deb60f94b5f22b1a0abd82c5dbb3fa5495

[Russell Taylor]

Re #5876. Always return output workspace of binary operation.

Always return the result workspace for compound binary operations as it's possible it could be a new workspace underneath (if a Workspace2D has changed size).

Changeset: 73b5b6d74bbe5ccc9b3693509f9973d74a584feb

[Stuart Campbell]

This ticket has been transferred to github issue ​6722