Skip to content
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

Change dtype of drs4 baseline values to int16 to make it work with pre-corrected EVBv6 data. #1182

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Nov 21, 2023

I repeat here what was discussed via email:

In EVBv5, similarly to what is done now, data was shifted up by 400 to allow for negative values after the pre-calibration by EVB.
But in the EVBv5 data, this offset wasn't specified, so it wasn't subtracted automatically. Instead, we computed the relative DRS4 offset coefficients on this shifted data, which resulted in drs4 coefficients around 400, subtracting this offset and
also resulting in drs4 coefficients being positive (we store them as uint16 then).

With the new data format, the offset and scale are included and always applied by the source immediately when reading.
This results in values around 0, so the drs4 coefficient script also computes drs4 offsets around 0.

This would all be fine, if we wouldn't store them as uint16, clipping the values below 0.

So my proposal would be to change to int16 for the drs4 offsets. Since we have a 12 bit ADC, that's not an issue, we still can store all possible values, also for not-yet applied pre-calibration by EVB. The binary representation also won't change, since all values up to now have been positive and < 2**15.

So in short, I think we should adapt the computation script to store values as int16 and issue the warning only in case of non-precalibrated drs4 values.

@maxnoe
Copy link
Member Author

maxnoe commented Nov 21, 2023

Tested on the evbv6 night, gives this distribution of drs4 offsets, as expected close around 0:

evb6_baseline

@maxnoe
Copy link
Member Author

maxnoe commented Nov 24, 2023

Please don't merge yet, I didn't yet test if applying these corrections like this actually works.

@maxnoe maxnoe marked this pull request as draft November 24, 2023 16:03
@moralejo
Copy link
Collaborator

moralejo commented Dec 4, 2023

@maxnoe Any news on this?
We will need it for EvB v6 data even if the difference won't be large...

@maxnoe
Copy link
Member Author

maxnoe commented Dec 4, 2023

I tested with a standard drs4 evbv5 run, it yields the expected values around 400.

I didn't yet check in any meaningful way the application of the calibration to evbv6 data, but I don't expect any issues there, since the data with the clipped drs4 corrections was already ok.

@maxnoe maxnoe marked this pull request as ready for review December 4, 2023 11:17
@moralejo
Copy link
Collaborator

moralejo commented Dec 4, 2023

I tested with a standard drs4 evbv5 run, it yields the expected values around 400.

I didn't yet check in any meaningful way the application of the calibration to evbv6 data, but I don't expect any issues there, since the data with the clipped drs4 corrections was already ok.

Ok, thanks!

@moralejo moralejo merged commit 089891d into main Dec 4, 2023
7 checks passed
@moralejo moralejo deleted the drs4_evb6 branch December 4, 2023 13:03
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.

2 participants