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

Set module coords to 0 for jungfrau detector of one module. #206

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kakhahmed
Copy link

@kakhahmed kakhahmed commented Feb 16, 2023

This MR fixes an issue when plotting xarray for FXE_XAD_JF500K which has one module JNGFR03.

This fix is used by https://git.xfel.eu/calibration/pycalibration/-/merge_requests/775

EDITED:

More info as the description was too short.

This issue happens after creating components.JUNGFRAU object for FXE_XAD_JF500K to form a DataArray for data.adc and the plot it using plot_data_fast. The problem is that the module coordinates for this DataArray are of a value 3. As the data channel has JNGFR03.

Screenshot-from-2023-02-14-12-18-18

@takluyver
Copy link
Member

I'm not sure this is quite right: if they called the solo module '1', would the 2 modules of JF1M be '2' and '3'?

We should check if any other instruments have done strange bits of numbering like this - if not, maybe the best thing is to special-case it in the EXtra-data component for the name FXE_XAD_JF500K.

@kakhahmed
Copy link
Author

if they called the solo module '1', would the 2 modules of JF1M be '2' and '3'

Yes. So the same detector-type modules have an incrementing module number. For example, we can have the same issue with AGIPD if there is another added AGIPD at MID or SPB. The module numbers will start from 16,...

But what is the assumption this code is based on? That we can have two Jungfraus in the same instrument using JNFR01?

Adding a special case should work I guess, But I don't think it's a long-term solution, unfortunately.

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