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

FDC uses corrected position #1254

Merged
merged 1 commit into from Sep 5, 2023
Merged

FDC uses corrected position #1254

merged 1 commit into from Sep 5, 2023

Conversation

shenyangshi
Copy link
Contributor

See https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenonnt:analysis:signals_and_detector:20230815:fdc, now straxen uses original positions for the FDC correction but the map is developed for z_dv_corr.

Note that the SR0 result won't be affected as the z_bias_map in SR0 is a dummy map so z_corr=0.

I also delete some fields that don't make sense to me.

@shenyangshi shenyangshi marked this pull request as ready for review September 4, 2023 19:21
@shenyangshi
Copy link
Contributor Author

The change is minimal, but the logic will be consistent.
image

@dachengx
Copy link
Collaborator

dachengx commented Sep 4, 2023

The change is minimal, but the logic will be consistent. image

What is the data here?

@shenyangshi
Copy link
Contributor Author

It's 043256 with the master branch of straxen and global_v12 correction https://github.com/XENONnT/corrections/pull/219

@dachengx
Copy link
Collaborator

dachengx commented Sep 4, 2023

So can we explain the difference of r in your demo by the change from z_naive to z_dv_corr? Do we actually expect a change, or is the reason for the change only digital precision?

@shenyangshi
Copy link
Contributor Author

I don't know, something is not fully reproducible, but the change is negligible.
image

@shenyangshi
Copy link
Contributor Author

Do you want to do a test by yourself? Corrections and versions in my folder are changing rapidly, I cannot promise I didn't make any stupid mistake in checking.

@dachengx
Copy link
Collaborator

dachengx commented Sep 5, 2023

Using environment:

^ Awesome table ^^^^
^ module ^ version ^ path ^ git ^
| python | 3.9.17 | /opt/XENONnT/anaconda/envs/XENONnT_2023.07.2/bin/python | None |
| strax | 1.5.3 | /home/xudc/strax/strax | branch:master | a466a94 |
| straxen | 2.1.2 | /home/xudc/straxen/straxen | branch:correct_fdc | f2f8825 |
| cutax | 1.15.3 | /home/xudc/cutax/cutax | branch:master | 4aa29c9 |

and code

run_id = '043256'

st = cutax.xenonnt_offline()

st.set_config(
    {'fdc_map': 'itp_map://resource://'
     'XnT_3D_FDC_xyz_MLP_v1.2_B2d75n_C2d74n_G0d3p_A5d0p_T0d9n_PMTs1d3n_FSR0d65p.json.gz'
     '?fmt=json.gz&scale_coordinates=plugin.coordinate_scales'}
)

EventPositions0 = st.get_single_plugin(run_id, 'event_positions')

I get
image

@dachengx
Copy link
Collaborator

dachengx commented Sep 5, 2023

And I do not think you should use &scale_coordinates=plugin.coordinate_scales.

@shenyangshi
Copy link
Contributor Author

Yes indeed there is some change... Maybe due to interpolation errors. We don't have to use &scale_coordinates=plugin.coordinate_scales if the map itself is in z but not in drift time, but unfortunately in my reprocessings the url config has this, so I have to make the samething in the new plugin to compare.

Copy link
Collaborator

@dachengx dachengx left a comment

Choose a reason for hiding this comment

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

Then please check a bit. :)

Copy link
Contributor

@jingqiangye jingqiangye left a comment

Choose a reason for hiding this comment

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

Hey I did some checks and it looks good to me.

@dachengx
Copy link
Collaborator

dachengx commented Sep 5, 2023

Hey I did some checks and it looks good to me.

Professional!

@dachengx dachengx merged commit 2e9eaaa into master Sep 5, 2023
9 of 10 checks passed
@dachengx dachengx deleted the correct_fdc branch September 5, 2023 21:01
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.

None yet

3 participants