New ELUT correction and propagation of live time uncertainty#244
New ELUT correction and propagation of live time uncertainty#244paolomassa wants to merge 5 commits intoi4Ds:masterfrom
Conversation
…files and propagation of livetime uncertainty.
New calibration files are now used for spectroscopy (only for CPD files, not for spectrograms for now)
ennosigaeus
left a comment
ennosigaeus
left a comment
There was a problem hiding this comment.
A few observations on the new calibration file search logic and the ELUT correction flow — mostly clarifications and one question about intent. Nice work on the overall restructuring.
|
|
||
| elut_data = stx_elut_correction(this_count_rates, this_count_rates_error, $ | ||
| energy_bin_idx, energy_bin_low, energy_bin_high, energy_high, energy_low, e_bin, this_energy_range, $ | ||
| spectrum, pixel_ind, det_ind, /silent) |
There was a problem hiding this comment.
Note (det_ind is fine here): Initially flagged this as passing a string ('top24') to stx_elut_correction, but det_ind is already converted to numeric indices at line 129 via stx_label2det_ind. No issue.
One minor observation: det_ind is the requested detectors while detectors_used (line 221) is the intersection of requested and actually-present detectors. Inside stx_elut_correction, subc_index only affects the diagnostic outputs (.COUNTS_NO_ELUT, .COUNTS_ELUT), not the main corrected counts. So this works correctly — just noting the distinction.
| default, plot, 1 | ||
| default, det_ind, 'top24' | ||
| default, elut_correction, 1 | ||
| default, elut_correction, 1 |
There was a problem hiding this comment.
Cleanup (dead variable): elut_correction is set here but never referenced anywhere in this procedure — the correction logic is now driven by keyword_set(calib_data). Could be removed to avoid confusion. Low priority.
| @@ -161,9 +202,12 @@ pro stx_convert_pixel_data, fits_path_data = fits_path_data, fits_path_bk = fit | |||
| distance = fits_distance, time_shift = time_shift, fits_path_bk = fits_path_bk, uid = uid, $ | |||
| generate_fits = generate_fits, specfile = specfile, srmfile = srmfile, elut_file = elut_filename, silent = silent) | |||
There was a problem hiding this comment.
Question (elut_filename may be undefined here): elut_filename is passed to stx_fits_info_params at this line, but it is only assigned at line 260 inside if keyword_set(fits_path_bk). When no background file is provided, elut_filename is undefined here.
No crash occurs because stx_fits_info_params defaults elut_file to '' — but it means the ELUT filename is never recorded in the FITS output metadata for the no-background case.
Is this intentional? If the ELUT filename should always be recorded, consider moving the stx_date2elut_file call (currently at line 260) above this line so it runs unconditionally.
ennosigaeus
left a comment
ennosigaeus
left a comment
There was a problem hiding this comment.
Two more observations — one on code duplication and one on an edge case in the spectral index calculation.
| aux_fits_file = aux_fits_file, flare_location_hpc = flare_location_hpc, flare_location_stx = flare_location_stx, $ | ||
| eff_ewidth = eff_ewidth, sys_uncert = sys_uncert, plot = plot, background_data = background_data, silent = silent, $ | ||
| elut_correction = elut_correction, fits_info_params = fits_info_params, xspec=xspec, ospex_obj = ospex_obj | ||
| ;;***************** CREATE SRM |
There was a problem hiding this comment.
Observation (SRM/OSPEX duplication): The SRM construction and OSPEX integration (lines 552–700, ~150 lines) is now inlined here. The old stx_convert_science_data2ospex.pro still exists and was not modified in this PR.
This means there are now two copies of the SRM/OSPEX logic — the new inline version (with the updated ELUT correction, grid transmission, and livetime propagation) and the old standalone function. Fixes to one won't propagate to the other.
Not blocking, but worth discussing: should stx_convert_science_data2ospex be deprecated, or should the shared logic be extracted so both paths call the same code?
|
|
||
| for i=0,idx_peak-1 do begin | ||
| this_x=alog(e_mean(i+1))-alog(e_mean(i)) | ||
| this_y=alog(spectrum(i+1))-alog(spectrum(i)) |
There was a problem hiding this comment.
Edge case (zero or negative spectrum values): If any spectrum bin is zero or negative after background subtraction, alog(spectrum(i)) produces -Inf or NaN. The cleanup at lines 107–108 (where(~finite(...))) catches bad values in the final index_final array, but the intermediate e_weighted values (computed from index_tmp in the first pass, lines 64–66) could already contain NaN. Since e_weighted feeds the second-pass recalculation (lines 87–101), NaN can propagate to adjacent good bins via alog(e_weighted(i-1)).
The flat-spectrum case (all equal values) is fine — produces all-zero indices as expected. The concern is specifically about bins with zero or negative counts.
- Remove committed .DS_Store and add it to .gitignore - Fix date typos: "April 22026" → "April 2026", "Fubruary" → "February" - Add missing trailing newlines to five new files
ennosigaeus
force-pushed
the
new_calib_data
branch
from
fa1a4a4 to
58ed4ab
Compare
2 participants
We implemented a new ELUT correction which
Correction has been implemented for imaging and for spectroscopy. However, in the case of spectroscopy it works only for CPD files and not for spectrograms (ELUT correction for spectrograms is meaningless).
We also added code for propagating live time uncertainty.
Finally, we use a calibrated version of the BKG detector transmission in
stx_convert_pixel_data. This calibrated transmission has been provided by @MurielStiefel.TODO list:
stx_convert_spectrogramas it is or modify it in a way similar to what done forstx_convert_pixel_data?