Skip to content

Updates for v4.0#180

Open
haiderh1970 wants to merge 3 commits intoEMVA1288:masterfrom
haiderh1970:master
Open

Updates for v4.0#180
haiderh1970 wants to merge 3 commits intoEMVA1288:masterfrom
haiderh1970:master

Conversation

@haiderh1970
Copy link

I added some stuff to be more compliant with v4.0 of the standard:

  • LE_min and LE_max was replaced by LE in the report
  • Stability check was added as plot
  • Additional run for dark-current measurement was added (allowing you to use longer exposure times with fewer points). I tried to upload a respective dataset, but the data limit is already reached.
    The data describing dark current would look like this in the emva1288.txt:

DarkCurrent run with 6 images

dc 10000.0
i DarkCurrent/Img_10-0.png
i DarkCurrent/Img_10-1.png
dc 200008000.0
i DarkCurrent/Img_200008-0.png
i DarkCurrent/Img_200008-1.png
dc 400006000.0
i DarkCurrent/Img_400006-0.png
i DarkCurrent/Img_400006-1.png
dc 600004000.0
i DarkCurrent/Img_600004-0.png
i DarkCurrent/Img_600004-1.png
dc 800002000.0
i DarkCurrent/Img_800002-0.png
i DarkCurrent/Img_800002-1.png
dc 1000000000.0
i DarkCurrent/Img_1000000-0.png
i DarkCurrent/Img_1000000-1.png

  • Small adjustments to the tests to still work.

Copy link

@MichaelWright44 MichaelWright44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I like the idea of adding stability curves, and updating linearity error to be more consistent, nice work!

self.data = {}
self.data['temporal'] = self._get_temporal(data['temporal'])
self.data['spatial'] = self._get_spatial(data['spatial'])
self.data['darkcurrent'] =self._get_temporal(data['darkcurrent'], True)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.data['darkcurrent'] =self._get_temporal(data['darkcurrent'], True)
self.data['darkcurrent'] = self._get_temporal(data['darkcurrent'], True)

print()

def _get_temporal(self, data):
def _get_temporal(self, data, dark_only=False):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't mind adding dark_only to the Parameters section in _get_temporal

array of the digital dark value variance for each exposure time.
*'s2_ydark'*: the
array of the digital dark value variance for each exposure time
and *'diff_u_y'*: the difference of the mean values of the images

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep s2_ydark and diff_u_y separated in the doc string here

Suggested change
and *'diff_u_y'*: the difference of the mean values of the images
*'s2_ydark'*: the array of the digital dark value variance
for each exposure time.
*'diff_u_y'*: the difference of the mean values of the images
for each exposure time.

Comment on lines +215 to +216
marker='o',
markersize=5,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This marker style makes it a little difficult to track deviation from fit, I think sticking to default plotting params for the data is fine

Suggested change
marker='o',
markersize=5,

image

Comment on lines +247 to +248
marker='o',
markersize=5,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same idea as above

Suggested change
marker='o',
markersize=5,

image

@fariza
Copy link
Contributor

fariza commented May 23, 2025

Hi, I'm really sorry for my super non excusable late comeback
If you are still interested we can retake the review of these changes

@haiderh1970
Copy link
Author

Dear Federico.
I guess we will perform some updates anyhow, to align with the latest version of the standard.
But anyhow, I would appreciate if you check these updates and maybe merge them.

@fariza
Copy link
Contributor

fariza commented Jun 16, 2025

@haiderh1970 I have to do a change of the data loading side of things, the current state is pretty old, ugly and unreliable.

To avoid modifying the data loading can we limit the scope to the first two points?

  • LE_min and LE_max was replaced by LE in the report
  • Stability check was added as plot
    We can add back the dark current once the data loading has been replaced

@haiderh1970
Copy link
Author

Sounds good

@latif1414
Copy link

By when is it planned to fix the warnings of the tests or switch to revision 4.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants