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

Fixed bug in selection of energy indices of pixel data matrix #185

Merged
merged 3 commits into from
Nov 2, 2023
Merged

Fixed bug in selection of energy indices of pixel data matrix #185

merged 3 commits into from
Nov 2, 2023

Conversation

paolomassa
Copy link
Collaborator

There was a potential mismatch between the input energy range and the indices of the energy axis that were selected in the pixel data matrix. The problem raised when e_axis.LOW started from 0 keV (where e_axis is the energy axis structure returned by stx_read_pixel_data_fits_file). Indeed, in that case, the indices corresponding to the input energy interval were selected assuming that the first energy bin was 0-4 keV, while the first energy interval of the pixel data matrix (stored in the counts variable) was systematically discarded before selecting the energy bins. As a consequence, it could happen that the input energy interval was 5-8 keV, but the energy interval that was actually selected by the code was 7-9 keV.

A similar mismatch could also happen when the indices of the ELUT were selected. In that case, the energy correction factors of the first and the last energy interval (which should be between ~0.95 and ~1.05) could take values close to 2 or close to 0 (or even negative!).

I solved this issue in the following way:

  • I added energy_bin_mask among the fields contained in the data structure returned by stx_read_pixel_data_fits_file. In this way, we have information on which energy bins are contained in the science (or background) fits file
  • I use the energy_bin_mask to select the energy bins of data.COUNTS (where data is the structure returned by stx_read_pixel_data_fits_file) that actually contains measurements. Therefore, I assume that data.COUNTS always contains 32 energy bins and that the energy bins that do not contain actual measurements are filled with zeros. @grazwegian, is this assumption correct?
  • As for the ELUT, the number of energy bins contained in the matrix returned in ekev_actual is 31 (@grazwegian, this valid for every ELUT, right?). Therefore, when I create the matrices containing the low and the high energy edges, I create them so that they contain 32 energy bins by setting the low edge of the first energy bin equal to zero and the high edge of the last energy bin equal to NaN.
  • The implementation also assumes that both the science and the background fits files contain more that one energy bin (i.e., that the energy_bin_mask contains more than one entry equal to one).

Fixed potential mismatch between the input energy range and the indices of the energy axis that were selected in the pixel data matrix. First release.
@paolomassa paolomassa marked this pull request as ready for review October 16, 2023 22:13
@paolomassa
Copy link
Collaborator Author

I have tested the changes I made and they seems to be correct. Also Säm tried out the code and approved the changes.

@grazwegian
Copy link
Collaborator

  • Yes, an array of 32 (energies) x 12 (pixels) x 32 (detectors) x number of time bins is always generated by stx_read_pixel_data_fits_file and then populated with the data in the file see: https://github.com/i4Ds/STIX-GSW/blob/d87eb573ad40ca6b7354df1c0733b0a12080d3d2/stix/idl/io/stx_read_pixel_data_fits_file.pro#L167C9-L167C9

  • The ELUT files contain the 31 edges to describe the 30 normal science energy channels. The telemetry additionally contains edges at ADC values of 0 and 4095 to define the lower and upper level discriminator bins to bring the total to 32. It is unlikely although possible that this could be changed in the future (e.g. if we wanted to use all 32 bins in the science range) in that case there would need to be changes to the software in many places.

@grazwegian
Copy link
Collaborator

It would probably be good to discuss the expectations around stx_read_pixel_data_fits_file at the software meeting in Poland.

@samaloney
Copy link
Collaborator

  • The implementation also assumes that both the science and the background fits files contain more that one energy bin (i.e., that the energy_bin_mask contains more than one entry equal to one).

I think it's possible to have only one entry in the energy_bin_edge_mask e.g ``0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]` is 4-5 keV

@paolomassa
Copy link
Collaborator Author

I think the code crashes if only one energy bin is present. Do you want me to modify it so that it handles also that case?

@samaloney samaloney merged commit 8bcb623 into i4Ds:master Nov 2, 2023
grazwegian added a commit to grazwegian/STIX-GSW that referenced this pull request Nov 2, 2023
* i4ds:
  Fixed bug in selection of energy indices of pixel data matrix (i4Ds#185)
grazwegian added a commit to grazwegian/STIX-GSW that referenced this pull request Nov 22, 2023
* i4ds:
  Fixed bug in selection of energy indices of pixel data matrix (i4Ds#185)
  Some Improvement in aspect processing (i4Ds#179)
grazwegian added a commit to grazwegian/STIX-GSW that referenced this pull request Nov 22, 2023
* default:
  Bugfix release
  stx_read_fits : uncommented call to mrdfits to prevent compilation issue when getting version number
  preparing a new release
  Fixed bug in selection of energy indices of pixel data matrix (i4Ds#185)
  stx_subc_transmission: Improved accuracy of message displayed if simple_transm keyword is set.

# Conflicts:
#	stix/idl/io/stx_read_fits.pro
#	stix/idl/processing/spectrogram/stx_fsw_sd_spectrogram2ospex.pro
grazwegian added a commit to grazwegian/STIX-GSW that referenced this pull request Dec 14, 2023
* i4ds: (28 commits)
  Bugfix release
  stx_read_fits : uncommented call to mrdfits to prevent compilation issue when getting version number
  preparing a new release
  Fixed bug in selection of energy indices of pixel data matrix (i4Ds#185)
  stx_subc_transmission: Improved accuracy of message displayed if simple_transm keyword is set.
  Update by Massa, P. - use simplified version of the subcollimator transmission for imaging (temporary solution)
  Some Improvement in aspect processing (i4Ds#179)
  Initial release of basic routines for downloading L2 AUX ephemeris files given a date and L1 science data fits files given a UID. (i4Ds#189)
  Fixed bug in the estimation of the total number of bkg counts (i4Ds#186)
  stx_livetime_fraction: update of default eta value using empirical high trigger rate data (i4Ds#188)
  Energy calibration improvement (i4Ds#183)
  Call lightcurve without generating FITS files (i4Ds#181)
  Respect shift_duration if file is "possibly summed on board". (i4Ds#180)
  Clarify flare_location coordinates (i4Ds#169)
  changed default behaviour of flux keyword
  Improvements to transmission following suggestions from @paolomassa. Value of flux = 1 passed through from  stx_fsw_sd_spectrogram2ospex. Additional description of calculation of  linear attenuation coefficient in stx_subc_transmission.
  stx_write_ospex_fits: fix grid factor recording in FITS file for srm file
  stx_write_ospex_fits: fix grid factor recording in FITS file
  stx_subc_transmission:  added default to calculate low energy approximation if no input photon energies are passed
  fix calculation for single included grid
  ...

# Conflicts:
#	stix/idl/processing/spectrogram/stx_convert_pixel_data.pro
#	stix/idl/processing/spectrogram/stx_convert_science_data2ospex.pro
#	stix/idl/processing/spectrogram/stx_convert_spectrogram.pro
#	stix/idl/processing/spectrogram/stx_science_data_lightcurve.pro
grazwegian added a commit to grazwegian/STIX-GSW that referenced this pull request Mar 21, 2024
* main: (45 commits)
  Add silent keyword and print ELUT name (i4Ds#201)
  Update of "stx_estimate_flare_location" (i4Ds#209)
  Automatic formatting
  Update VERSION.txt
  Lightcurve demo fix (i4Ds#208)
  Spectroscopy demo fix (i4Ds#207)
  fixed i4Ds#205 (fix by Dominic Zarro)
  Update VERSION.txt
  Update stx_imaging_demo (i4Ds#203)
  Remove unused keyword from structure
  Add IDL_STARTUP file to stix/setup which will be run every time SSWIDL is started. Currently this only calls stx_check_config_files (i4Ds#196)
  Skip ELUT (i4Ds#182)
  Bugfix release
  stx_read_fits : uncommented call to mrdfits to prevent compilation issue when getting version number
  preparing a new release
  Fixed bug in selection of energy indices of pixel data matrix (i4Ds#185)
  stx_subc_transmission: Improved accuracy of message displayed if simple_transm keyword is set.
  Update by Massa, P. - use simplified version of the subcollimator transmission for imaging (temporary solution)
  Some Improvement in aspect processing (i4Ds#179)
  Initial release of basic routines for downloading L2 AUX ephemeris files given a date and L1 science data fits files given a UID. (i4Ds#189)
  ...

# Conflicts:
#	stix/idl/processing/spectrogram/stx_convert_pixel_data.pro
#	stix/idl/processing/spectrogram/stx_convert_spectrogram.pro
#	stix/idl/processing/spectrogram/stx_fsw_sd_spectrogram2ospex.pro
#	stix/idl/processing/spectrogram/stx_write_ospex_fits.pro
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.

3 participants