ml_norm for virtual crystals #833

gefeichen · 2021-02-24T06:42:07Z

I got it by dividing normalized projdata by raw projdata (mCT).
And no zero value found in

STIR/src/utilities/find_ML_normfactors3D.cxx

Line 162 in 882c261

measured_fan_data(ra,a,rb,b) = 0;

after make_fan_data_remove_gaps

I modified ml_norm3d code with virtual crystal, i.e., make_fan_data_remove_gap, set_fan_data_add_gap, ML_norm. new_max_delta in ML_norm.cxx may be wrong.

gefeichen · 2021-02-24T07:19:07Z

And another image: upper is a cylinder sinogram, below is a sinogram after normalisation

KrisThielemans

This looks very nice to me. Thanks a lot.

I have 2 suggestions in the review below. In addition, I think we should get rid of the old make_fan_data and its inverse. Otherwise we keep too much code duplication.

Add yourself as author and whoever owns your copyright in the headers. Also, add a note in release_5.0.htm in section changed functionality.

KrisThielemans · 2021-02-24T07:38:01Z

src/buildblock/ML_norm.cxx

+    const int num_physical_transaxial_crystals_per_block = num_transaxial_crystals_per_block - virtual_transaxial_crystals;
+
+    const int num_physical_axial_crystals_per_block = num_axial_crystals_per_block - virtual_axial_crystals;


I guess we could just as well provide the member functions for these 2 as well.

KrisThielemans · 2021-02-24T07:45:00Z

src/buildblock/ML_norm.cxx

-                    int new_b = b - b/num_transaxial_crystals_per_block;
-                    int new_ra = ra - ra/ num_axial_crystals_per_block;
-                    int new_rb = rb - rb/num_axial_crystals_per_block;
+                    (*segment_ptr)[bin.axial_pos_num()][bin.view_num()][bin.tangential_pos_num()] = 0;


I suggest we use an extra input argument to the function gap_value, defaulting to 0. It will also make it clearer what's happening here.

KrisThielemans · 2021-02-24T08:01:04Z

I believe this supersedes your other PR. If so, please close it by referring to this one.

I don't understand the first picture. Can you provide a bit more detail?

gefeichen · 2021-02-24T08:21:02Z

I believe this supersedes your other PR. If so, please close it by referring to this one.

I don't understand the first picture. Can you provide a bit more detail?

I used a cylinder source in gPET to cover the whole FOV and generated corresponding model projection data.

It's hard to visualize the norm parameter. So I just use manip_segments to divide model_data by measure_data. And the result shows in the first picture. Maybe it's useless.

KrisThielemans · 2021-02-24T08:25:55Z

easier to look at the inverse of it:

stir_math -s --including-first --power -1 eff_factors.hs norm_factors.hs

could do the trick

KrisThielemans · 2021-02-24T08:29:40Z

I'd also compare measured data with model*eff_factors. I think you get the latter with apply_normfactors3D with apply_or_undo=1.

gefeichen · 2021-02-24T09:44:04Z

The absolute difference of measure and model*eff_factors is 19%
sum(abs(model*eff-measure))/sum(measure)
I don't know whether it is correct.

Below is eff and geo factors. left:eff right:geo

KrisThielemans · 2021-02-24T10:29:17Z

The absolute difference of measure and model*eff_factors is 19%

they will be different because of noise of course. Hard to quantify that. But 19% doesn't look too bad. efficiencies look a bit strange but could be alright.

I note that my text was confusing. with "eff_factors" I meant "eff*geo" (i.e. the total efficiency, also known as "1/norm_factors"). I'll call them "total_eff_factors" below.

Things to compare measured with model*total_eff_factors:

visual comparison of
profiles. you will probably have to do some averaging to reduce noise.
reconstruction of the gate data used for normalisation (lm_to_projdata with excluding scatters and randoms, no background in recon).
reconstruction of other gate data (lm_to_projdata with excluding scatters and randoms, no background in recon).
reconstruction of other gate data full pipeline (although that doesn't have anything to do with the norm)

Are you using https://github.com/UCL/STIR-GATE-Connection? If so, it tells you how to handle the norm (currently BinNormalisationFromProjData needs the norm-factors, not the total_eff_factors)

KrisThielemans

can you merge master in here to resolve the conflict? sorry.

KrisThielemans · 2021-02-24T10:31:07Z

src/buildblock/ML_norm.cxx

 /// **** This function make proj_data from fan_data while adding the intermodule gaps **** ////
 /// *** fan_data doesn't have gaps, proj_data has gaps *** ///
 void set_fan_data_add_gaps(ProjData& proj_data,
-                  const FanProjData& fan_data)
+                  const FanProjData& fan_data, const int gap_value)


float please

KrisThielemans · 2021-02-24T10:31:50Z

src/include/stir/ML_norm.h

 void set_fan_data_add_gaps(ProjData& proj_data,
-                  const FanProjData& fan_data);
+                  const FanProjData& fan_data,
+                  const int gap_value=0);


Suggested change

const int gap_value=0);

const float gap_value=0.F);

# Conflicts: # src/buildblock/ML_norm.cxx # src/include/stir/ML_norm.h

KrisThielemans · 2021-03-02T18:41:07Z

@gefeichen, will your mCT tests be affected by #838 and/or #837?

NikEfth · 2021-03-07T03:00:08Z

Hi , I am sorry for the delay, I will test with my data tonight.

NikEfth

Hi @gefeichen
Thank you for your good work,
I don't have mCT data but from my data (which as we had discussed are slightly different ) (and your code) I saw that the last segment was coming out empty.
I don't think that this actually has to do with your modifications but, could you please check you last, and second to last segments? I believe that those would be +54 and +53 (?), might be wrong, though.

NikEfth · 2021-03-15T14:33:10Z

src/buildblock/ML_norm.cxx

+    const int num_axial_blocks_in_max_delta = max_delta/(num_axial_crystals_per_block);
+    const int new_max_delta = max_delta - (num_axial_blocks_in_max_delta)*virtual_axial_crystals;
+    const int num_physical_detectors_per_ring = num_detectors_per_ring - num_transaxial_blocks*virtual_transaxial_crystals;
+    const int num_physical_rings = num_rings - (num_axial_blocks-1)*virtual_axial_crystals;


I agree with this.

gefeichen · 2021-03-16T06:46:02Z

Hi @NikEfth
Thanks for your checking.
Could you provide more information about your data? Like ring numbers, virtual crystal number, and position

KrisThielemans · 2021-09-22T19:15:41Z

This seems ready to me now, and we have the signed letter.

@NikEfth if you have any final comments, let us know. Otherwise I'll merge

gefeichen added 2 commits October 14, 2020 15:57

Hi

5c2dc09

I modified ml_norm3d code with virtual crystal, i.e., make_fan_data_remove_gap, set_fan_data_add_gap, ML_norm. new_max_delta in ML_norm.cxx may be wrong.

ml_norm part with new code

0d9f5d4

gefeichen marked this pull request as ready for review February 24, 2021 06:43

gefeichen changed the title ~~Test~~ ml_nom_virtual_crystal Feb 24, 2021

KrisThielemans requested a review from NikEfth February 24, 2021 07:51

KrisThielemans requested changes Feb 24, 2021

View reviewed changes

KrisThielemans changed the title ~~ml_nom_virtual_crystal~~ ml_norm for virtual crystals Feb 24, 2021

gefeichen mentioned this pull request Feb 24, 2021

virtual crystal for ml_norm 3d #713

Closed

ml_norm part with new code

78d00e4

KrisThielemans requested changes Feb 24, 2021

View reviewed changes

gefeichen and others added 6 commits March 1, 2021 15:42

ml_norm part with new code

77dc260

ml_norm part with new code

be2608f

Merge remote-tracking branch 'origin/test' into test

242f472

# Conflicts: # src/buildblock/ML_norm.cxx # src/include/stir/ML_norm.h

ml_norm part with new code

4c704bb

Merge branch 'master' into test

55397a8

minor update to release notes

75137ae

KrisThielemans approved these changes Mar 1, 2021

View reviewed changes

robbietuk mentioned this pull request Mar 15, 2021

ML norm code should work by bucket, not by block #838

Open

NikEfth reviewed Mar 15, 2021

View reviewed changes

Merge branch 'master' into test

cb7f82b

KrisThielemans merged commit 301cf1e into UCL:master Sep 24, 2021

KrisThielemans mentioned this pull request Oct 1, 2021

Virtual xtal interface. #776

Open

KrisThielemans mentioned this pull request Oct 23, 2021

ML_norm is broken since #833 #948

Closed

robbietuk mentioned this pull request May 4, 2022

Example mMR geometry may be out of date? UCL/STIR-GATE-Connection#88

Open

KrisThielemans mentioned this pull request Sep 20, 2023

add virtual crystal support for ml_norm #691

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ml_norm for virtual crystals #833

ml_norm for virtual crystals #833

gefeichen commented Feb 24, 2021

gefeichen commented Feb 24, 2021

KrisThielemans left a comment

KrisThielemans Feb 24, 2021

KrisThielemans Feb 24, 2021

KrisThielemans commented Feb 24, 2021

gefeichen commented Feb 24, 2021

KrisThielemans commented Feb 24, 2021

KrisThielemans commented Feb 24, 2021

gefeichen commented Feb 24, 2021

KrisThielemans commented Feb 24, 2021

KrisThielemans left a comment

KrisThielemans Feb 24, 2021

KrisThielemans Feb 24, 2021

KrisThielemans commented Mar 2, 2021

NikEfth commented Mar 7, 2021

NikEfth left a comment

NikEfth Mar 15, 2021

gefeichen commented Mar 16, 2021

KrisThielemans commented Sep 22, 2021

		const int num_physical_transaxial_crystals_per_block = num_transaxial_crystals_per_block - virtual_transaxial_crystals;

		const int num_physical_axial_crystals_per_block = num_axial_crystals_per_block - virtual_axial_crystals;

ml_norm for virtual crystals #833

ml_norm for virtual crystals #833

Conversation

gefeichen commented Feb 24, 2021

gefeichen commented Feb 24, 2021

KrisThielemans left a comment

Choose a reason for hiding this comment

KrisThielemans Feb 24, 2021

Choose a reason for hiding this comment

KrisThielemans Feb 24, 2021

Choose a reason for hiding this comment

KrisThielemans commented Feb 24, 2021

gefeichen commented Feb 24, 2021

KrisThielemans commented Feb 24, 2021

KrisThielemans commented Feb 24, 2021

gefeichen commented Feb 24, 2021

KrisThielemans commented Feb 24, 2021

KrisThielemans left a comment

Choose a reason for hiding this comment

KrisThielemans Feb 24, 2021

Choose a reason for hiding this comment

KrisThielemans Feb 24, 2021

Choose a reason for hiding this comment

KrisThielemans commented Mar 2, 2021

NikEfth commented Mar 7, 2021

NikEfth left a comment

Choose a reason for hiding this comment

NikEfth Mar 15, 2021

Choose a reason for hiding this comment

gefeichen commented Mar 16, 2021

KrisThielemans commented Sep 22, 2021