oslocyclotronlab / ompy_Archive_Sept2019

Archived version om ompy as of 27. Sept 2019 -- Please use this instead:
https://github.com/oslocyclotronlab/ompy/
GNU General Public License v3.0
0 stars 1 forks source link

Bug in new FG -- wrong normalization matrix #27

Closed fzeiser closed 5 years ago

fzeiser commented 5 years ago

Issue by fzeiser Friday Jul 26, 2019 at 13:13 GMT Originally opened as https://github.com/oslocyclotronlab/ompy/issues/27


As described in the commit message, there is a bug in the FG method in 169190e6de657ff7ee45500f4069c393d8421511.

We have seen that the normalization matrices are different to what they were in the method implemented before. A part of this could be here:

https://github.com/oslocyclotronlab/ompy/blob/169190e6de657ff7ee45500f4069c393d8421511/ompy/firstgeneration.py#L70-L71

This is not exactly the same as https://github.com/oslocyclotronlab/ompy/blob/169190e6de657ff7ee45500f4069c393d8421511/ompy/first_generation_method.py#L87-L93

due to lower bin vs bin center calibration. Don't know which one we want here though! Erlends solutions seems to make sense to me!

fzeiser commented 5 years ago

Comment by fzeiser Friday Jul 26, 2019 at 13:34 GMT


Even when one cuts the matrices the same way, there was a difference in the "areas": They seemed to be wrong in "jørgen" implementation. When I changed to multiplicity_estimation="total", the difference for the number of counts disappears. -

fzeiser commented 5 years ago

Comment by fzeiser Friday Jul 26, 2019 at 15:39 GMT


When proceeding with multiplicity_estimation="total" it seems like following matrices are the same in iteration 0:

Other comments

vs

https://github.com/oslocyclotronlab/ompy/blob/169190e6de657ff7ee45500f4069c393d8421511/ompy/first_generation_method.py#L391-L393

If I include the reshape in firstgeneration.py everything seems to work quite smoothly. You can then get the difference between the two matrices. Let's plot it here when it's all fixes (need to include the fill_negatives before the comparison is meaningful).

A quick diff (but implemented several things hacky !!) looks like this :+1: (so there are probably some more differences, if you only change the last line with the rebining things) diff

As I did several changes I'll just upload the two files I used, in case we want to trace back the differences. Most apparent changes are just print/plot statements. (as txt due to upload requirements)

firstgeneration.py.txt first_generation_method.py.txt

fzeiser commented 5 years ago

Comment by fzeiser Friday Jul 26, 2019 at 15:47 GMT


@Caronthir: Maybe good to look at this thread before you start on Monday.

And a reminder for me: Another implementation for the reshape would be W = div0(W, W.sum(axis=1)[:,None]

fzeiser commented 5 years ago

Comment by fzeiser Tuesday Sep 03, 2019 at 08:37 GMT


Fixed. I think it was in dcc78515d48e8298bcf46118ed72809cdfd7eb9a