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

Binary conversions for CALET #20

Merged
merged 5 commits into from
Aug 7, 2024
Merged

Conversation

athish-thiru
Copy link
Contributor

Binary conversions for GECAM mission. Note

  • Does not currently convert detector information (lack of documentation)
  • Does does store record_number/alert_type/alert_datetime/$schema

@athish-thiru athish-thiru changed the title GECAM binary conversion Binary conversions for multiple missions Jul 30, 2024
Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run black to fix the CI pipeline failures.

gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to run black...

gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
@lpsinger
Copy link
Member

lpsinger commented Aug 1, 2024

Remember to run black...

The lint checks are still failing. #21 will automate the code formatting for you once it's merged.

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like a PR that adds a parser for one notice type.

@athish-thiru
Copy link
Contributor Author

For future reference, do you want an individual PR for each notice type?

@athish-thiru athish-thiru changed the title Binary conversions for multiple missions Binary conversions for CALET Aug 2, 2024
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
"trigger_type": "rate",
"rate_energy_range": [energy_range_data[1], energy_range_data[0]],
"rate_snr": bin_arr[9] * 1e-2,
"rate_duration": bin_arr[11] * 1e-2,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation calls this field bkg_dur1. Field 10 is fore_dur. Which one corresponds to rate_duration?

Copy link
Contributor Author

@athish-thiru athish-thiru Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I chose bkg_dur1 is that the documentation defines it as "the amount of time (units centi-sec) used to determine the background countrate when used for the lc_significance" and the descriptions for rate_duration is "Interval over rate signal to noise ratio calculation". Although thinking about it again I believe bkg_dur1 is being used to calculate the noise, so should I switch to fore_dur?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jracusin, @Vidushi-GitHub, would you please check this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of it corresponds to rate_duration, as that's defined as signal to noise ratio. CALET has defined "bkg_dur1" as interval before the trigger, fore_dur as triggering interval.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation defines lc_signif as "signal-to-noise ratio (in units of centi-sigma) during the foreground interval". Are foreground interval and triggerring interval different?

gcn_classic_to_json/utils.py Outdated Show resolved Hide resolved
@lpsinger
Copy link
Member

lpsinger commented Aug 5, 2024

@athish-thiru, would you please rebase from main again?

@athish-thiru
Copy link
Contributor Author

@lpsinger I believe I have rebased to the new main now

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix pipeline failures.

gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/utils.py Outdated Show resolved Hide resolved
@athish-thiru
Copy link
Contributor Author

athish-thiru commented Aug 6, 2024

I also want to note that the url(http://cgbm.calet.jp/cgbm_trigger/flight/1399729386/index.html) doesn't link to anything. I've added for completeness since the fields do contain data and the url is present in the text notices as well.

gcn_classic_to_json/utils.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/CALET_GBM_FLT_LC/example.json Outdated Show resolved Hide resolved
@lpsinger
Copy link
Member

lpsinger commented Aug 6, 2024

I also want to note that the url(http://cgbm.calet.jp/cgbm_trigger/flight/1399729386/index.html) doesn't link to anything. I've added for completeness since the fields do contain data and the url is present in the text notices as well.

@jracusin, @Vidushi-GitHub, would you please track down what the correct URLs are for the CALET notices?

lpsinger and others added 4 commits August 7, 2024 10:24
Explicit int conversion

removed MAXI_UNKNOWN conversion

Removed GECAM_FLT and GECAM_GND conversion

Added documentation and changed function name

reformatted file

deleted files
@athish-thiru
Copy link
Contributor Author

athish-thiru commented Aug 7, 2024

I didn't add asserts for these since:

  • packet_sod changes for every packet
  • packet hopcount should be 0 for recent notices, but I presume that as you go back through the archive at some point it stops being zero
  • packet sernum is mostly 1 in practice but there could be some notices which were sent out minutes apart that may have an incremental value

@lpsinger lpsinger merged commit 284ff23 into nasa-gcn:main Aug 7, 2024
2 checks passed
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.

None yet

3 participants