-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix bug when µMUX data has no number of rows stored in LJH file #300
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -251,14 +251,18 @@ def _parse_header(self): | |
# Read the new (Feb 2024) subframe information. If missing, assume the old TDM values pertain | ||
# (so # of rows -> subframe divisions, and row # -> subframe offset), unless source is Abaco, | ||
# in which case use 64 subframe divisions and offset of 0. | ||
default_divisions = self.number_of_rows | ||
default_offset = self.row_number | ||
default_divisions = self.number_of_rows | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is the only place we should have a fix, and it should have a comment explaining why it works the way it does, and probably print a warning about what is wrong that is making it need the fix. Perhaps "warning: malformed ljh filed with number_of_rows = 0 detected, doing x instead". |
||
if default_divisions <= 0: | ||
default_divisions = 1 | ||
if "Abaco" in self.source: | ||
# The external trigger file can override this, but assume 64 divisions at first. | ||
default_divisions = 64 | ||
default_offset = 0 | ||
self.subframe_divisions = int(header_dict.get(b"Subframe divisions", default_divisions)) | ||
self.subframe_offset = int(header_dict.get(b"Subframe offset", default_offset)) | ||
if self.subframe_divisions <= 0: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a redundant fix, if the above fix works, this isn't necessary. So future readers will have a hard time understanding why this exists and it will make reading the code harder without adding any value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue title says that the issue happane when no number of rows is stored in the LJH file. But we have a line should make |
||
self.subframe_divisions = 1 | ||
|
||
self.version_str = header_dict[b'Save File Format Version'] | ||
self.binary_size = os.stat(filename).st_size - self.header_size | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
#LJH Memorial File Format | ||
Save File Format Version: 2.2.0 | ||
Software Version: MATTER Client 1.1.2 | ||
Software Driver Version: unknown | ||
Server Start Time: 16 Nov 2017, 19:12:14 GMT | ||
Acquisition Mode: 0 | ||
Digitized Word Size in Bytes: 2 | ||
File First Record Time: 16 Nov 2017, 22:23:34 GMT | ||
Channel: 3 | ||
Number of columns: 1 | ||
Column number (from 0-0 inclusive): 0 | ||
Location: LANL | ||
Cryostat: | ||
Bias level (% Rn): 1.1 V | ||
T set point (mK): 80 | ||
"Heater" output (%): | ||
Magnetic field (mg): | ||
Sample: | ||
Photon source: Gd153 | ||
Operator(s): | ||
System comment: | ||
#End of description | ||
Number of Digitizers: 1 | ||
Number of Active Channels: 1 | ||
Timestamp offset (s): 1510859534.658107 | ||
Digitizer: 1 | ||
Description: n/a | ||
Master: yes | ||
Bits: 16 | ||
Effective Bits: unknown | ||
Anti-alias low-pass cutoff frequency (Hz): n/a | ||
Timebase: 1.600000e-05 | ||
Number of samples per point: 1 | ||
Presamples: 125 | ||
Total Samples: 500 | ||
Digitizer Channel: 1.0 | ||
Dummy: 0 | ||
Range: 0.500000 | ||
Offset: 0.500000 | ||
Coupling: DC | ||
Impedance: n/a | ||
Inverted: No | ||
Preamp gain: 1.0 | ||
Discrimination level: n/a | ||
#End of Header | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 | ||
01234567890abcdef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This set of changes seems totally uneccesary and actually bad. We found this error because mass crashed when something went wrong, by giving default values instead of
None
values to all these parameters, we increase the likelyhood mass will do something silently wrong instead of crashing. These changes aren't needed to fix the bug at hand.