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

SNEWS conversions #38

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

SNEWS conversions #38

wants to merge 11 commits into from

Conversation

athish-thiru
Copy link
Contributor

Binary conversion for SNEWS

gcn_classic_to_json/notices/SNEWS/__init__.py Show resolved Hide resolved
gcn_classic_to_json/notices/SNEWS/__init__.py Outdated Show resolved Hide resolved
gcn_classic_to_json/notices/SNEWS/__init__.py Show resolved Hide resolved
Comment on lines 44 to 54
comments = ""
comments += (
"This is an Individual detection.\n"
if trig_id_bits[0] == 1
else "This is an Coincidence detection.\n"
)
comments += (
"This is a test notice.\n"
if trig_id_bits[1] == 1
else "This is a real notice.\n"
)
Copy link
Member

Choose a reason for hiding this comment

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

By transforming the bit field to comments, we are making it less machine readable than before. Please introduce schema definitions for these fields.

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 test notice is encapsulated in 'alert_tense' so I could remove that. However, I don't think there is a schema definition of individual or coincidence detection, so I would have to define a new field. I mainly added these since the comments in the text notices had these as well.

Copy link
Member

Choose a reason for hiding this comment

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

Please work with @Vidushi-GitHub to start defining schema where necessary to capture these fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spoke with @Vidushi-GitHub last week and we worked out a format to encode the detection quality and remove the comments. We also decided that the individual/coincidence detection was redundant and since this can be inferred from detector quality and I've left a comment in the code highlighting this

Comment on lines 58 to 59
if trig_id_bits[5] == 1:
comments += "This is definitely not a core-collapse Supernova."
Copy link
Member

Choose a reason for hiding this comment

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

Please capture this flag in a machine-readable way, not as a comment.

Actually if this bit is set, then the intention is to retract the alert. @Vidushi-GitHub, how do we represent a retraction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can modify the 'alert_type' field for this correction.

Copy link
Member

Choose a reason for hiding this comment

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

The following field is missing:

long         9       event_flue   [counts]        Number of neutrinos (0 to infinity)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This data is stored in 'n_events'. I defined a new field since there is no equivalent in the core schema.

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.

2 participants