[Ecat2Nii] Only apply ECAT_CALIBRATION_FACTOR if it has not already b…

[noahg-neuroimage]

…een applied; check CALIBRATION_UNITS to see if calibration has been applied, if it has, return input image, otherwise multiply by calibration factor.

This change will correctly account for whether the calibration factor has already been applied to Ecat data. In the current version of PET2BIDS, ecat2nii will erroneously reapply the calibration factor on data that has already been calibrated, resulting in Nifti files with incorrect data objects.

---

📚 Documentation preview 📚: https://pet2bids--268.org.readthedocs.build/en/268/

[bendhouseart]

Tagging @CPernet as this would include changes to the similarly functioning ecat2nii.m.

[CPernet]

What BIDS field is that? All the ecat we have seen are scaled but there is no 'calibration factor' as far as I remember - happy to change things.

[bendhouseart]

Scaling is always applied during the conversion for ecat to nii. I'm not sure if that's because we chose to follow the same conventions as used when converting from dicom to nifti or if it was just the agreement we came to during one of our discussions. That said, one could always undo that scaling by dividing the image by the DoseCalibrationFactor as they are one in the same.

I don't think we were aware that the ecat calibration units denoted whether to scale or not to. Most of our ecat knowledge derives from the following snippets from the ECAT 6.3 and ECAT 7.2 manuals, which aren't particularly clear to me about scaling or not scaling:

You're the first person to approach is with any knowledge of this ancient file format, so we'd be happy to make any corrections that you're able to make us aware of.

[CPernet]

dose calibration is not the same as the image scaling -- this happens at two different locations, which is why I am asking what are we talking about
-- image scaling as per scanner https://github.com/openneuropet/PET2BIDS/blob/main/matlab/ecat2nii.m#L157
-- calibration factor https://github.com/openneuropet/PET2BIDS/blob/main/matlab/ecat2nii.m#L281

image scaling can not be changed - would make no sense, calibration factor could -- but how do you know in the ecat data if this was apply? i have not see a field that tells anything about that -- I have never seen CALIBRATION_UNITS at 1
-- before merging we need to check our ecat validator

[noahg-neuroimage]

Coming back to this, if you can take a look at p9816fvat1_fbp_fixed.nii.gz, and look at the later frames- the voxel values for the background artifacts far exceed those inside the brain. Typically we might see a ratio of 10-1000 from the brightest brain voxels to the background artifacts in a later frame using our previously trusted software. PET2BIDS is doing something funny to the values, and I can't confirm necessarily that the change I made for this PR is the correct change, so I'd like your opinion on how the FBP image looks before proceeding.

[bendhouseart]

@noahg-neuroimage I haven't shared the ecat's with anyone yet for comparison, with your permission I can forward them securely via email to @CPernet and @mnoergaard.

[CPernet]

@noahg-neuroimage I would need to see the ecat file - not the nifti - are you fine with sharing privately via @bendhouseart

[noahg-neuroimage]

@CPernet I confirmed with @bendhouseart last week permission to share the Ecat files with you. Can you confirm whether that was communicated?

[bendhouseart]

@noahg-neuroimage I sent over only the nifti's like a blockhead, @CPernet and @mnoergaard now have the original ecat's as well.

[bendhouseart]

Just to confirm, applying the fix suggested in this PR does lead to the correct scaling for the images in question. I've attached an image showing the original ecat viewed in vinci (I don't know how to make it larger so it's easier to inspect, so you may have to take my word on the next part) compared to the nifti output from the PR as viewed in FreeView. And not applying the ECAT_CALIBRATION_FACTOR leads to the images in each format to display the same values per voxel/region:

I need to double check and be a bit more thorough than a visual inspection with vinci and Freeview. However, what's interesting, is that when I manually set the ECAT_CALIBRATION_FACTOR to 1 we get the following output:

I was hoping this was a one off fix and that a user could just supply pet2bids with --kwargs ECAT_CALIBRATION_FACTOR=1 but it looks we do apply the scaling factor twice in the Python version. The next step is to check this with the Matlab version to see if we get the same results.

[bendhouseart]

I think I found the issue, see:

https://github.com/openneuropet/PET2BIDS/blame/86a8f7e78a2c28c84f033f08b191d23d53f4e7f3/pypet2bids/pypet2bids/ecat2nii.py#L190

[CPernet]

@bendhouseart i don't think we should play with the slope of the nifti - instead we should apply or not directly on the data
just like @noahg-neuroimage proposes if header x then
-- in matlab the conditional statement should be here https://github.com/openneuropet/PET2BIDS/blob/main/matlab/ecat2nii.m#L281
-- which is different from playing with the nifti header (which would be here https://github.com/openneuropet/PET2BIDS/blob/main/matlab/ecat2nii.m#L363)

[noahg-neuroimage]

I'll share that previous methods we've used result in the nifti header's scl_slope set to nan. I'd presume that in this version of the code, when the calibration factor is explicitly applied, and then set in scl_slope, the result is the calibration is applied twice. This would be ignorant to whether the calibration factor has already been applied to the data, in which case the factor is applied a total of three times.

[mnoergaard]

I agree with @CPernet. In readECAT7.m there is an optional input argument to the function, where it is possible to specify whether the calibration should be 'on'. It is per default set to being uncalibrated i.e. 'off'. When it is specified as 'on', the lines of code on 183-186 are activated

if calibrated,  % convert to double pixels and scale to mh.data_units
            cf = sh{m}.scale_factor*mh.ecat_calibration_factor;  % calibration factor for mh.data_units
            data{m} = cf*data{m};
        end; 

since we are doing this last step outside readECAT7.m in ecat2nii.m as pointed out by @CPernet, we should apply the conditional statement here.

[CPernet]

@mnoergaard actually it was few lines below we needed to add the conditional statement -- info is the json I we should? report that value not matter what

@bendhouseart just to be super transparent to users, I now always issue a statement about the state of the dose calibration

see #271

[bendhouseart]

Hey all, I went ahead and checked out the branch patch-2 as well as the the branch containing the Matlab changes @CPernet pushed to #271 and ran both python and matlab ecat conversions. Additionally I converted the original ecat's with Turku's ecat2nii as a sanity check. I "tagged" all of the nifti's with the source used to generate them. e.g. I used the commit hash and matlab/python for pet2bids and tagged the others with "Turku".

It's probably easier to describe this with a table, so I'll edit or reply to this comment after I calculate, say, the average or total value of frame X for the original ecat's, turku ecat2nii output, and pet2bids ecat2nii output. In the meanwhile feel free to examine the nifti's labeled as described that I'm about to share with you.

[CPernet]

overkill sounds good - thx! i guess mean square differences across the entire image would be good

[mnoergaard]

I am still not quite happy with the output from pet2bids after these recent changes. After carrying out some more testing, I can reproduce the outputs from Turku (i.e. ecat2nii), which are also in line with the ecat viewer in Vinci. But to get the same outputs as Turku, I do indeed need to apply the ecat_calibration_factor despite calibration_units being set to 1 (= calibrated), which seems counterintuitive. So that suggests that 1=calibrated means that calibration should be carried out, and 0=uncalibrated means that calibration should not be carried out? Or are the Turku ecat2nii and Vinci wrong? @CPernet and @bendhouseart I will send you some code to reproduce the behavior.

[CPernet]

I understood it the other way around, but as mentioned on slack I am also very confused about that

[mnoergaard]

Well, that is also how I think 99.9% would understand it (including me), but based on the tests with Turku and Vinci, 'calibrated' means 'not calibrated and thus should be calibrated'.

And so just to be clear, based on the test data that we have received (p9816fvat1_osem.ecat), calibration_units=1 suggesting that the data has NOT been calibrated, and hence SHOULD be calibrated.

[noahg-neuroimage]

I can confirm that, after some digging, the code we use for file conversion from Ecat at my institution does apply the calibration factor without an option not to apply it. The issue I brought up here, it turns out has to do with the python version doubly applying the calibration via scl_slope, rather than relating to CALIBRATION_UNITS in the header, at least from the user's perspective.

[noahg-neuroimage]

Additionally, I looked at the header info on some of our Ecat phantom scans, and these scans also have CALIBRATION_UNITS=1, which may not make sense for a phantom. This field may not be informative the way I initially suggested, it's even possible our scanner was set with CALIBRATION_UNITS=1 as a default value and not changed on a per-scan basis.

EDIT: For the sake of sharing information, I checked the header of an Ecat sinogram file. The unconstructed sinogram header has ECAT_CALIBRATION_FACTOR=0 and CALIBRATION_UNITS=0. So it seems the CALIBRATION_UNITS field is changed during reconstruction.

[bendhouseart]

I don't know if it all makes sense to me, but it is becoming clear. I mentioned to Cyril and Martin in another discussion, but my best guess is that someone at Siemens wanted to code the calibration factor scaling such that:

ECAT_CALIBRATION_FACTOR=ECAT_CALIBRATION_FACTOR*ECAT_CALIBRATION

It seems like we're all on the page it this point.

[bendhouseart]

Alright, the current state of the PR contrasted with some of the previous commits, the important take away is that *python_1_is_apply* matches up with the same matlab fix in #271.

[bendhouseart]

👍

[noahg-neuroimage]

I'm struggling with running this fork as an independent instance of pypet2bids in order to test with my data. What's the preferred way to do this?

[bendhouseart]

I'm struggling with running this fork as an independent instance of pypet2bids in order to test with my data. What's the preferred way to do this?

Hmm, I've been using the github cli to checkout and push changes to your branch via this PR:

gh pr checkout 268

You should be able to git checkout patch-2 && git pull, then run make buildpackage && make installpackage to run it locally from your python environment.

[bendhouseart]

@noahg-neuroimage if it's easier you should be able unzip (github won't let me upload a .whl file) and then pip install the attached file to test out the changes from this branch:

pypet2bids-1.3.4-py3-none-any.whl.zip

[noahg-neuroimage]

I've had the chance to do some testing. It seems to work fine on OSEM images, and values within the brain for FBP look fine but values outside the brain in FBP still seem off. These seem to match up with the negative valued voxels in the original FBP ECAT image. As far as I know, negative values are valid in FBP reconstructions, however this version of ecat2nii takes the negative values in the FBP image and scales them to brighter than the brain values, which will cause issues during processing.

Can the PET2BIDS team confirm whether this issue occurs when converting the supplied FBP ECAT file under the current version of ecatpet2bids?

[bendhouseart]

Hmm, are you sure that your FBP images shouldn't look like oversized digital TV static?

There's some slight differences between the matlab and python images, but the general issue you're describing seems applicable to both. That said, I'm going to close this PR and open up a new issue specific to just FBP as we are making forward progress.

[bendhouseart]

@noahg-neuroimage closing this PR as the original issue has been addressed and branches are all diverging, see issue #272 for continued discussion relating to FBP. Also, @CPernet @mnoergaard you've been notified too.

[CPernet]

@mnoergaard tracked the error and I remember we 'fixed' it because of negative values 11dca7a obviously we were wrong, mostly because we thought it was not making sense -- for to update the python reader for negative values @bendhouseart an I'm guessing reverting the matlab one

[CPernet]

9beee53