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

Tool to produce a time calibration HDF5 file #600

Closed
wants to merge 24 commits into from
Closed

Conversation

Bultako
Copy link
Collaborator

@Bultako Bultako commented Feb 23, 2021

This PR is the continuation of #533 developed in a forked repo and now with the code placed in a branch of lstchain repo.

It adds a new Tool lstchain_create_time_calibration_file

Generate a HDF5 file with time calibration coefficients

Options
=======
The options below are convenience aliases to configurable class-options,
as listed in the "Equivalent to" description-line of the aliases.
To see all configurable class-options for some <cmd>, use:
    <cmd> --help-all

--debug
    Set log-level to debug, for the most verbose logging.
    Equivalent to: [--Application.log_level=10]
--show-config
    Show the application's configuration (human-readable format)
    Equivalent to: [--Application.show_config=True]
--show-config-json
    Show the application's configuration (json format)
    Equivalent to: [--Application.show_config_json=True]
-q, --quiet
    Disable console logging.
    Equivalent to: [--Tool.quiet=True]
-i, --input=<Path>
    Path to the fits.fz events file or directory to glob
    Default: None
    Equivalent to: [--TimeCalibrationHDF5Writer.input]
-o, --output=<Unicode>
    Path to the time calibration file
    Default: ''
    Equivalent to: [--TimeCorrectionCalculate.calib_file_path]
--glob=<Unicode>
    Filename pattern to glob files in the directory
    Default: '*.fits.fz'
    Equivalent to: [--TimeCalibrationHDF5Writer.glob]
--max-events=<Int>
    Maximum number of events that will be read from the file
    Default: None
    Equivalent to: [--LSTEventSource.max_events]
--pedestal=<telescopeparameter-item-1>...
    Path to the LST pedestal file
    Default: [('type', '*', None)]
    Equivalent to: [--LSTR0Corrections.drs4_pedestal_path]
--run-summary-path=<telescopeparameter-item-1>...
    Path to the run summary for the correct night. If given, dragon reference
    counters are read from this file. Explicitly given values override values
    read from the file.
    Default: [('type', '*', None)]
    Equivalent to: [--EventTimeCalculator.run_summary_path]
--config=<Path>
    name of a configuration file with parameters to load in addition to command-
    line parameters
    Default: None
    Equivalent to: [--Tool.config_file]
--log-level=<Enum>
    Set the log level by value or name.
    Choices: any of [0, 10, 20, 30, 40, 50, 'DEBUG', 'INFO', 'WARN', 'ERROR', 'CRITICAL']
    Default: 30
    Equivalent to: [--Tool.log_level]
-l, --log-file=<Path>
    Filename for the log
    Default: None
    Equivalent to: [--Tool.log_file]
--log-file-level=<Enum>
    Logging Level for File Logging
    Choices: any of [0, 10, 20, 30, 40, 50, 'DEBUG', 'INFO', 'WARN', 'ERROR', 'CRITICAL']
    Default: 'INFO'
    Equivalent to: [--Tool.log_file_level]

To see all available configurables, use `--help-all`.

@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.91%. Comparing base (96ae165) to head (777cea2).
Report is 1980 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #600   +/-   ##
=======================================
  Coverage   84.91%   84.91%           
=======================================
  Files          72       72           
  Lines        5541     5541           
=======================================
  Hits         4705     4705           
  Misses        836      836           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Bultako Bultako changed the title Tool to produce a time calibration HDF5 file. Tool to produce a time calibration HDF5 file Feb 23, 2021
@Bultako Bultako marked this pull request as draft February 25, 2021 15:33
@Bultako
Copy link
Collaborator Author

Bultako commented Mar 4, 2021

I guess that for testing this PR we need bigger raw files than those already present in test_data. I have the exception below when testing with LST-1.1.Run02008.0000_first50.fits.fz

Caught unexpected exception: Not enough events to coverage all capacitor. 
Please use more events to time calibration file.

@maxnoe
Copy link
Member

maxnoe commented Mar 4, 2021

@Bultako is that an option? Could you run the test with less required events?

And maybe fix the language of the error?

@rlopezcoto
Copy link
Contributor

@Bultako is that an option? Could you run the test with less required events?

And maybe fix the language of the error?

For the time calibration it is complicated because you need to cover all capacitors to be able to calculate the time correction for all of them. In principle the exception is right and you should not be able to produce a time calibration file without covering all capacitors.
I cannot think on any way of performing tests without that, but maybe @FrancaCassol or @pawel21 have some idea.

@maxnoe
Copy link
Member

maxnoe commented Mar 4, 2021

We can also try to look for the smallest possible file.

Unfortunately that will be around 1000 events or so I think.

@Bultako Bultako force-pushed the tool-timecalib branch 7 times, most recently from 5057216 to 9487a07 Compare March 6, 2021 09:40
@rlopezcoto
Copy link
Contributor

this one was reviewed and approved, could you re-approve @maxnoe so we can also include it in the next release?

@maxnoe
Copy link
Member

maxnoe commented Dec 13, 2021

I think we need to adapt the onsite scripts so that actually the new tool is called, right?

@rlopezcoto
Copy link
Contributor

I would get this tool in the release before, and then lstosa can be later updated to use it

@maxnoe
Copy link
Member

maxnoe commented Dec 13, 2021

I am not talking about osa, I am talking about the onsite scripts in this repo.

@FrancaCassol
Copy link
Collaborator

I think we should concentrate towards what is blocking release 0.8.0
This tool should replace the script, so I think it should be merged when the lstchain script is removed and the onsite script is adapted. But I will personally not approve any further change in the calibration scripts before 0.8.0

@rlopezcoto
Copy link
Contributor

ok, let's leave this for lstchain > v0.8

@vuillaut
Copy link
Member

Should be closed?

@maxnoe
Copy link
Member

maxnoe commented Sep 10, 2024

Yes, all the calibration related tools have been moved to the new lstcam-calib project, we shouldn't update the ones here

@vuillaut vuillaut closed this Sep 11, 2024
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.

5 participants