Skip to content
Snippets Groups Projects

[Jungfrau][Correct] Account for missing modules by depending on `xarray`.

Open Karim Ahmed requested to merge fix/account_for_missing_jf_module into master
3 unresolved threads

Description

This shows a problem that can be faced when not all expected modules' data are available http://max-exfl-cal001.desy.de:8008//gpfs/exfel/exp/XMPL/202150/p700002/usr/Reports/r0164/FXE_XAD_JF1M_correct_700002_r0164_240521_132426_322759.pdf

For example for JF1M in case JNGFR01 is missing, an error is raised by extra-geom for attempting to plot data with unexpected data shape.

This MR introduces keep_data_dims function which does something similar to the keep_dims boolean when set for extra-data reader. unfortunately, such functionality is not available for components. So I added this solution in cal_tools.

How Has This Been Tested?

  • Running Automated tests

Relevant Documents (optional)

jf1m_xmpl.pdf

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • I added tests where appropriate.

Reviewers

@kluyvert @schmidtp

Edited by Karim Ahmed

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • assigned to @ahmedk

  • Karim Ahmed added 1 commit

    added 1 commit

    Compare with previous version

    • If I've understood this correctly, there is actually a way to do this with components, but not with the .trains() iterator. Something like:

      jf['data.adc'].select_trains(0).ndarray(module_gaps=True)

      The module_gaps parameter here means to create the array sized for all the expected modules. However, to use this reliably, we would need to tell the JUNGFRAU component how many modules are expected - n_modules=2 in this case.

      Another option is to get an xarray, which is labelled with the module number, rather than a NumPy array. EXtra-geeom can handle these even if some modules are missing.

      I'm happy to review this as is if you don't want to go down either of those routes, though.

    • Thank you, to be honest, I was depending on such insight as I wasn't sure if I missed an already available functionality that I could have used.

      I have updated the MR with your suggestion and testing it

    • Please register or sign in to reply
  • Karim Ahmed added 2 commits

    added 2 commits

    • d59052c4 - fix: no need to use keep_data_dims function and use available functionalities in extra
    • d90f7f18 - test: remove test and unneeded function

    Compare with previous version

  • Karim Ahmed changed the description

    changed the description

785 "\n",
786 "# TODO: replace with CALCAT value.\n",
787 "if \"1M\" in karabo_id:\n",
788 " nmods = 2\n",
789 "elif \"4M\" in karabo_id:\n",
790 " nmods = 8\n",
791 "else: # 500K\n",
792 " nmods = 1\n",
793 "\n",
794 "with DataCollection.from_paths(seq_corrected_files) as corr_dc:\n",
786 795 " # Reading CORR data for plotting.\n",
787 796 " jf_corr = components.JUNGFRAU(\n",
788 797 " corr_dc,\n",
789 798 " detector_name=karabo_id,\n",
799 " n_modules=nmods,\n",
790 800 " ).select_trains(np.s_[:plot_trains])\n",
  • Comment on lines 787 to 790

    Thinking about it, we might also need first_modno= for detectors like FXE's JNGFR03, otherwise .ndarray() will create a (1, ...) shaped array and then try to fill out[2]. :disappointed:

    Either that or we add a special case in the component for n_modules=1.

  • That is true. Both options (using n_modules or passing xarray) wont be complete with out passing first_modno

    So I either go back to my first implementation or work on updating jungfrau constant retrieval and we start using first_module_index, which I think is the proper long term solution.

    However I am not sure how feasible is it to start doing this now. From your experience with LPD and updating it. Do you think this will take to long to achieve as we are waiting for some new CALCAT features

  • Please register or sign in to reply
  • Karim Ahmed added 2 commits

    added 2 commits

    • 8ffe7a6c - fix: move to using xarray for simplicity
    • 525daec3 - fix: remove unneeded changes

    Compare with previous version

  • Karim Ahmed changed title from [Jungfrau][Correct] Account for missing modules. to [Jungfrau][Correct] Account for missing modules{+ by depending on xarray+}.

    changed title from [Jungfrau][Correct] Account for missing modules. to [Jungfrau][Correct] Account for missing modules{+ by depending on xarray+}.

  • Karim Ahmed added 1 commit

    added 1 commit

    • b8aae122 - remove unavailable da from karabo_da

    Compare with previous version

    • Hmmm, what do we do about this?

    • If I remember correctly, I preferred waiting for the possibility to have the metadata of the detector from CALCAT like the first module index and the number of modules.

      I haven't checked since how easy it is to add or if it is already available to use. I will remind myself later about this.

    • Please register or sign in to reply
  • Please register or sign in to reply
    Loading