European-XFEL / karabo_data

Python tools to read and analyse data from European XFEL
https://karabo-data.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
13 stars 7 forks source link

Jungfrau1 m geom #198

Open dallanto opened 5 years ago

dallanto commented 5 years ago

Started to push new branch and did pull request simply for code review purposes. Commits are for JF_500KGeometry wrt new default unit: metres in inspect method etc. @takluyver

dallanto commented 5 years ago

Hi Thomas, exposing the tentative code via Github online is easier for discussion. So, I multiplied by pixel_size, which practically means that I multiply corner_x (_y) by unit instead of px_conversion variable. This taken alone seems to have no effect. But I did not touch the fs_vec and ss_vec yet: it's not clear to me how this should be multiplied because it seems to be a mere unit vector for the direction, i. e. without scale anyway.

takluyver commented 5 years ago

ss_vec and fs_vec define the unit vector for moving from one pixel to the next, so they do have a scale. When it was in pixel units, the unit vector is 1 pixel long (by definition). When it's in metres, the unit vector is pixel_size metres. E.g. here for AGIPD:

https://github.com/European-XFEL/karabo_data/blob/68ee19d52cd7f140052d029545a7b6169ec9752a/karabo_data/geometry2/__init__.py#L636-L639

dallanto commented 5 years ago

ss_vec and fs_vec define the unit vector for moving from one pixel to the next, so they do have a scale. When it was in pixel units, the unit vector is 1 pixel long (by definition).

Allright, thanks for the explanation. Because the pixel unit is 1 it looked like a unit vector in the mathematical sense. Sorry for my rather gratuitous question (if questions can be such at all). I should anyway stick closer to the AGIPD example - I did so for the old JF draft in the first place - to familiarize myself with the new mechanism. I'd prefer the default pixel axes like with the AGIPD inspect display, and with the correct scaling the panel tags etc. should be placed correctly no matter which of the two units are chosen.

takluyver commented 5 years ago

+1 to having axis_units='px' the default for .inspect() - that would be consistent with all the existing geometry classes. I only switched it to use metres internally.

ebadkamil commented 5 years ago

There has been a push (redmine ticket from Dmitry) recently from FXE regarding assembled JungFrau1M image which they want to use for Azimuthal integration in karaboFAI. What is the timeline for this MR to get accepted? I may have to report today during the chapter meeting. No hurries

takluyver commented 5 years ago

@dallanto mentioned to me that he's working on other things with higher priority at the moment, and this isn't currently complete, so there's no current schedule for merging it. If it's required soon, we can either ask @dallanto to make it higher priority, or one of us can pick it up.

dallanto commented 5 years ago

@dallanto mentioned to me that he's working on other things with higher priority at the moment, and this isn't currently complete, so there's no current schedule for merging it. If it's required soon, we can either ask @dallanto to make it higher priority, or one of us can pick it up.

That's still the case: for me personally, completion of an ECM poster draft until Thursday (to give time for corrections) is highest priority. Below that, whether the AGIPD test code for karaboFAI or Jungfrau 1M in karabo_data have higher priority could probably be judged best by you, Ebad - kind of being involved in both requirements ...

ebadkamil commented 5 years ago

Thanks for the comments. I would first confirm today if we are aligned with the detector group regarding APPPENDER device to sent data from two JungFrau(now pulse resolved 12-16 pulses) modules as 4D numpy array as we have for other pulse resolved multi modular detectors. Then we can decide on the priority. If it is really very urgent (which I don't think so) I could simply stack these two modules in karaboFAI as a quick fix. Else we proceed as usual through karabo_data