SIRF error

[paskino]

SIRF Python: I am allocating an ImageData using the allocate. In CIL we can allocate just the memory, without setting it. I have done the same with a SIRF DataContainer but it fails. What I see is

python ../../../../buildVM/sources/SIRF/examples/Python/PETMR/PET_MCIR.py -T "fwd_tm_ms_*.txt" -t tm -S "raw_FDG_ms*hs"  -r "FGP_TV" -i 100 -s 100 --visualisations -v 0 -o spdhg --algorithm="spdhg"
Setting up acquisition models...
Setting up reconstructor...
Will output results to: spdhg.log
SPDHG setting up
wrong fill value. Should be ImageData, numpy.ndarray, float or int

Notice that the program ends without raising an exception but just printing to screen. So I don't have a clue where it all went wrong. There's a clue: it is a fill method, either ImageData.fill or AcquisitionData. Turns out to be the second.

In this particular case, we could change the raise error("wrong fill value") call with a

raise TypeError("Wrong fill value. Expecting ImageData, numpy.ndarray, float or int, got {}"\
.format(type(value))

I'd suggest that we use standard Python error types in our calls and create a new exception for errors that come from the engine: EngineError

[rijobro]

Is it also true that we shouldn't put our main in a try and catch, as this will hide the error stack:

try:
    main()
    print('done')
    exit(0)
except error as err:
    # display error information
    print('%s' % err.value)
    exit(1)               # <------ most of our scripts don't have this either

From what I understand, it should just be this:

if __name__ == "__main__":
    main()

[evgueni-ovtchinnikov]

@paskino: it looks like you wanted to call ImageData.allocate() with a string value (e.g. 'random') but made some spelling mistake - I can see no other reason for such output. Can I see your script that produced it?

[paskino]

It's buried inside the CIL where we allocate(None) to just allocate the memory.

For the time I've changed allocate to handle value=None as value=0. Now I see that allocate calls get_uniform_copy which is:

def get_uniform_copy(self, value = 0):
        ''' 
        Returns a true copy of this object filled with a given value;
        value:  a Python float.
        '''
        ad = AcquisitionData(self)
        ad.fill(value)
        ad.src = 'copy'
        return ad

Could we skip the fill step? This will fix it.

@rijobro explained why the code doesn't crash!

Still the suggestion of EngineError rather than error is valid and the use of more language exceptions hold.

[paskino]

@evgueni-ovtchinnikov in CIL we allow value as

• a number
• random or random_int
• None in which case the system just allocates the memory

[evgueni-ovtchinnikov]

@paskino: you can skip fill if self is not empty.

[paskino]

you mean if it's not a "template"? Is there a way to check that?

[evgueni-ovtchinnikov]

@paskino: make sure self.handle is not None and check self.dimensions()

[evgueni-ovtchinnikov]

@paskino: actually, my last suggestion does not work - empty templates have correct dimensions (otherwise they would be useless)!

I created branch detect-template, where AcquisitionData.number() is set to 0 if there is no data - see PR 807.

[KrisThielemans]

@evgueni-ovtchinnikov why do we need the fill if self is a template? I understand it for get_uniform_copy (otherwise there would be no data in it), but it seems that if you ask allocate(None), you shouldn't expect that there is any data there (and in fact it would be a good feature if it errors out if you'd try to get its data anyway).

So, I wonder if we shouldn't swap let stir.AcquisitionData.allocate just do the copy-constructor as opposed to calling get_uniform_copy (and then do the appropriate thing of course). We could then implement get_uniform_copy in terms of allocate (as the latter is the better choice anyway)

def allocate(self, value=0, **kwargs):
        """Alias to get_uniform_copy.
        CIL/SIRF compatibility
        """
        out = AcquisitionData(self)
        out.src = 'allocate'
        if value in ['random', 'random_int']:
           ...
        elif value = 'None'
          # don't do anything
       else
          out.fill(value)
      return out

If we rationalise this a bit further, we could even move all the if stuff to fill instead.

And if we're really lucky, we could put all this in DataContainer and only have fill being class-specific (not sure if there's a "virtual copy-constructor" in python).

[evgueni-ovtchinnikov]

If self is an empty template, out will also have no data, so no memory will be alocated.

[KrisThielemans]

sure, but if someone says allocate(value=None), surely they will want to fill it later, so I believe that is alright.

[evgueni-ovtchinnikov]

if one calls allocate they want to allocate, do they not?

[KrisThielemans]

not sure. They want to have an object to play around with. Do they care if has already been "allocated"? I wouldn't think so.

[evgueni-ovtchinnikov]

should be called differently then - allocate would be confusing

@paskino: since this is only needed for CIL, you are the one to resolve this

[KrisThielemans]

CIL won't be able to change the name for sure. I still feel that the name reflects the concept. The user doesn't need to know if there's some memory (or disk space) reserved or not.

[evgueni-ovtchinnikov]

@paskino: AcquisitionData.number() now returns 0 for templates.