Skip to content

Draft: Feat/independent cli/config independent

Robert Rosca requested to merge feat/independent-cli/config-independent into master

Takes the values from hardcoded dictionary xfel_calibrate.notebooks:notebooks and inserts them into the parameter cells of relevant notebooks, adds functions to parse notebooks for these variables and extract them.

To keep the current CLI behaviour of calling xfel-calibrate DETECTOR TYPE a dictionary xfel_calibrate.notebooks:aliases is kept, this dictionary maps detectors to types (actions), and types to a relative notebook path.

Description

More context and information here: https://git.xfel.eu/calibration/pycalibration/-/issues/78

Values from xfel_calibrate.notebooks:notebooks injected into notebooks via a script. For example for AGIPD-CORRECT:

notebooks = {
    "AGIPD": {
        "CORRECT": {
            "pre_notebooks": [
                "notebooks/AGIPD/AGIPD_Retrieve_Constants_Precorrection.ipynb"
            ],
            "notebook": "notebooks/AGIPD/AGIPD_Correct_and_Verify.ipynb",
            "dep_notebooks": [
                "notebooks/AGIPD/AGIPD_Correct_and_Verify_Summary_NBC.ipynb"
            ],
            "concurrency": {
                "parameter": "sequences",
                "use function": "balance_sequences",
                "default concurrency": [-1],
                "cluster cores": 16,
            },
        },
        ...
    },
    ...
}

The information in the innermost dictionary (pre_notebooks, dep_notebooks, etc...) was extracted from the dict and injected them into the parameters cell of the notebook:

pre_notebooks = ['notebooks/AGIPD/AGIPD_Retrieve_Constants_Precorrection.ipynb']
dep_notebooks = ['notebooks/AGIPD/AGIPD_Correct_and_Verify_Summary_NBC.ipynb']
concurrency = {'parameter': 'sequences', 'use function': 'balance_sequences', 'default concurrency': [-1], 'cluster cores': 16}

So all the information in the dictionary is preserved, reading the parameters cell lets you recreate an entry in the dictionary. The hardcoded dictionary xfel_calibrate.notebooks:notebooks is gone, but a small dictionary aliases remains which helps maintain the current CLI behaviour:

aliases: dict[Detector, dict[Action, str]] = {
    "AGIPD": {
        "DARK": "notebooks/AGIPD/Characterize_AGIPD_Gain_Darks_NBC.ipynb",
        "PC": "notebooks/AGIPD/Chracterize_AGIPD_Gain_PC_NBC.ipynb",
        ...,
    },
    "LPD": {
        "DARK": "notebooks/LPD/LPDChar_Darks_NBC.ipynb",
        "PC": "notebooks/LPD/Characterize_LPD_GAIN_CI_per_pixel_NBC.ipynb",
        "FF": "notebooks/LPD/LPD_FlatField_Radial_per_pixel_CI_NBC.ipynb",
    },
    ...
}

Previously the dictionary was imported in src/xfel_calibrate/nb_args.py like from .notebooks import notebooks, now that is replaced with from .notebooks import aliases as nb_aliases, this provides enough information for the CLI functions to run.

Since the aliases dictionary only contains the path to the notebook, additional details like pre_paths/dep_paths, concurrency, etc... are loaded in when required in the parse_argv_and_load_nb function. This is done with nb_info = notebooks.Notebook.from_path(notebook_path).dict.

This change has a few benefits for now:

  • Pre/post notebooks defined in the notebooks contents instead of separate file.
  • Removes need to have these parameters in the launchpad configs.
  • Replaces hardcoded notebooks small aliases dict that just maps detector/action to nb path.
  • Provides starting point for more generic processing.
  • Retains old interface.

Not implemented here, but easy to do later on:

  • Run notebooks not in the packaged notebook directory.
  • Allow for overriding of dep_notebooks/pre_notebooks/concurrency via CLI.

How Has This Been Tested?

  • TODO

Relevant Documents (optional)

  • TODO

Types of changes

  • Refactor (refactoring code with no functionality changes)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I added tests where appropriate.

Reviewers

  • todo after some testing.
Edited by Robert Rosca

Merge request reports