Lists of ci data analysis should be implemented at the attribute level

[Jammy2211]

Currently, if we have two charge injection images in a ci_analysis_data object, the list index is at the ci_analysis_data lelve, e.g.:

ci_analysis_data[0].image
ci_analysis_data[1].image
ci_analysis_data[0].noise_map
ci_analysis_data[1].noise_map

We should instead make it so that the list attributes are at the attribute level, e.g.:

ci_analysis_data_stack.image[0]
ci_analysis_data_stack.image[1]
ci_analysis_data_stack.noise_map[0]
ci_analysis_data_stack.noise_map[1]

I also would refer to a list of charge injections as a 'stack', where the attribute names are pluralized to signify it is a stack. We will retain a CIData class that represents a single charge injection image / data-set.

The reason we do not want to have the list index at the ci_analysis_data level is that in AutoLens we perform fits which fit the entire data-set (e.g. every image in the list) simultaneously. In these cases, we cannot simply apply a map function over the list of lens_data. A similar thing can happen in AutoCTI. For this reason, AutoFit has been designed so that it can be passed lists of images, noise-maps, etc, to perform the fit.

[rhayes777]

This is a bad idea. If you reordered the image list but not the noise list then you would introduce a very hard to find bug.

…

On 16 Jan 2019, at 14:14, James Nightingale ***@***.***> wrote: Currently, if we have two charge injection images in a ci_analysis_data object, the list index is at the ci_analysis_data lelve, e.g.: ci_analysis_data[0].image ci_analysis_data[1].image ci_analysis_data[0].noise_map ci_analysis_data[1].noise_map We should instead make it so that the list attributes are at the attribute level, e.g.: ci_analysis_data_stack.image[0] ci_analysis_data_stack.image[1] ci_analysis_data_stack.noise_map[0] ci_analysis_data_stack.noise_map[1] I also would refer to a list of charge injections as a 'stack', where the attribute names are pluralized to signify it is a stack. We will retain a CIData class that represents a single charge injection image / data-set. The reason we do not want to have the list index at the ci_analysis_data level is that in AutoLens we perform fits which fit the entire data-set (e.g. every image in the list) simultaneously. In these cases, we cannot simply apply a map function over the list of lens_data. A similar thing can happen in AutoCTI. For this reason, AutoFit has been designed so that it can be passed lists of images, noise-maps, etc, to perform the fit. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGETMt5683cunSCEb8Cz1p3M2fQY0yW2ks5vDzOvgaJpZM4aDJ_H>.

[rhayes777]

Obtaining a list of images is trivial: [data.image for data in ci_analysis_data] Please don’t go putting associated data into parallel lists just to avoid a list comprehension!

…

On 16 Jan 2019, at 14:16, richard hayes ***@***.***> wrote: This is a bad idea. If you reordered the image list but not the noise list then you would introduce a very hard to find bug. > On 16 Jan 2019, at 14:14, James Nightingale ***@***.*** ***@***.***>> wrote: > > Currently, if we have two charge injection images in a ci_analysis_data object, the list index is at the ci_analysis_data lelve, e.g.: > > ci_analysis_data[0].image > ci_analysis_data[1].image > ci_analysis_data[0].noise_map > ci_analysis_data[1].noise_map > > We should instead make it so that the list attributes are at the attribute level, e.g.: > > ci_analysis_data_stack.image[0] > ci_analysis_data_stack.image[1] > ci_analysis_data_stack.noise_map[0] > ci_analysis_data_stack.noise_map[1] > > I also would refer to a list of charge injections as a 'stack', where the attribute names are pluralized to signify it is a stack. We will retain a CIData class that represents a single charge injection image / data-set. > > The reason we do not want to have the list index at the ci_analysis_data level is that in AutoLens we perform fits which fit the entire data-set (e.g. every image in the list) simultaneously. In these cases, we cannot simply apply a map function over the list of lens_data. A similar thing can happen in AutoCTI. For this reason, AutoFit has been designed so that it can be passed lists of images, noise-maps, etc, to perform the fit. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub <#3>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGETMt5683cunSCEb8Cz1p3M2fQY0yW2ks5vDzOvgaJpZM4aDJ_H>. >

[Jammy2211]

Okay, I will keep it like that for now.

The problem is meshing this with a fit. If we fit two images, the fit itself cannot be a list, e.g:

fit[0].image
fit[1].image
fit[0].residual_map
fit[1].residual_map

The reason for this is because of something like an inversion, which fits all images simultaneously. Essentially, there are certain attributes which are then not indepedent across the list, so it is not true that you can write:

fit[0].inversion.matrix
fit[1].inversion.matrix

The matrix in this case becomes one giant matrix for the entire fit, thus it has to be:

fit.inversion.matrix

I think the best way to proceed is to store data as lists (e.g. ci_analysis_data[0]) and extract the image, noise-map etc. for the fit via a list comprehension. However, the fit itself uses those unpacked quantites to retain a structure like:

fit.image[0]
fit.image[1]
fit.residual_map[0]
fit.residual_map[1]
fit.inversion.matrix

[rhayes777]

OK, assuming that it is necessary to tie the image to a fit object in the way you suggest then we could do it like this: fit.inversion.matrix fit.ci_data_list[0].image fit.ci_data_list[0].noise fit.ci_data_list[1].image fit.ci_data_list[1].noise

…

On 16 Jan 2019, at 14:44, James Nightingale ***@***.***> wrote: Okay, I will keep it like that for now. The problem is meshing this with a fit. If we fit two images, the fit itself cannot be a list, e.g: fit[0].image fit[1].image fit[0].residual_map fit[1].residual_map The reason for this is because of something like an inversion, which fits all images simultaneously. Essentially, there are certain attributes which are then not indepedent across the list, so it is not true that you can write: fit[0].inversion.matrix fit[1].inversion.matrix The matrix in this case becomes one giant matrix for the entire fit, thus it has to be: fit.inversion.matrix I think the best way to proceed is to store data as lists (e.g. ci_analysis_data[0]) and extract the image, noise-map etc. for the fit via a list comprehension. However, the fit itself uses those unpacked quantites to retain a structure like: fit.image[0] fit.image[1] fit.residual_map[0] fit.residual_map[1] fit.inversion.matrix — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGETMvM9ANLHVxAYsOVWKLIbHxaY0UXSks5vDzrUgaJpZM4aDJ_H>.

[Jammy2211]

I am setting it up following the above syntax, using the class instance name ci_datas_fit.