-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lists of ci data analysis should be implemented at the attribute level #3
Comments
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>.
|
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>.
>
|
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 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 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] |
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>.
|
I am setting it up following the above syntax, using the class instance name ci_datas_fit. |
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.
The text was updated successfully, but these errors were encountered: