Expanding Core Schema for FRB #226
Conversation
Vidushi-GitHub
force-pushed
the
frb/specific
branch
2 times, most recently
from
84437ee to
5bf3f24
Compare
Vidushi-GitHub
force-pushed
the
frb/specific
branch
from
a243008 to
f80408b
Compare
There was a problem hiding this comment.
The changes I recommend are as follows:
- I would exclude the "dm_gal_ne_2001_max" from the core schema, as this value is an optional post detection analysis, and the specific model (i.e. NE_2001) may change or become obsolete.
- In the definition of spectral center, it is written that: "enum": ["energy", "wavelength", "frequency"] and "type": "number" which appears to be contradictory. I suggest removing the "enum": ["energy", "wavelength", "frequency"].
Note: I've made these changes to PR #217 so that my commits could pass the new consistency checks that this change introduces. I believe this PR can be moved/combined with PR #217.
Thank you, indeed helpful. |
|
@jracusin please review it. |
@tabbott36 please provide feedback/approve it. |
| "type": "string", | ||
| "enum": ["energy", "wavelength", "frequency"], | ||
| "description": "Mode of spectral measurement: high-energy or optical or radio observations, if not parsed, then default is energy", |
There was a problem hiding this comment.
Perhaps this should just be simplified into:
"Mode of spectral measurement; default is energy"
| "type": "string", | ||
| "enum": ["keV", "nm", "Hz"], | ||
| "description": "Units for the spectral data, if not parsed, then default unit is keV", |
There was a problem hiding this comment.
Do we need "if not parsed"?
| @@ -15,6 +15,10 @@ | |||
| "maxItems": 2, | |||
| "description": "Trigger time uncertainty [s, 1-sigma], with optional asymmetric uncertainty" | |||
| }, | |||
| "time_resolution": { | |||
| "type": "number", | |||
| "description": "Time resolution used for the source search [ms]" | |||
There was a problem hiding this comment.
How is this different from trigger_time_error?
There was a problem hiding this comment.
time_error is based on instrument time resolution.
time_resolution is choice based on transient type.
There was a problem hiding this comment.
Discretization is nothing more than a particular source of error. Why do we need a separate field for it?
| "items": { "type": "number" }, | ||
| "maxItems": 2, | ||
| "description": "Uncertainity associated with the dispersion measure [pc/cm^3, 1-sigma], with optional asymmetric uncertainty." |
There was a problem hiding this comment.
If the asymmetric uncertainty is optional, can you please accept either an array or a number?
Description
Observables relevant for radio transients:
Dispersion Measure schema
Reporter.schema.json:
spectral_centerDateTime.schema.json:
time_resolution