Support a list or a single number in localization #120

Vidushi-GitHub · 2023-12-29T01:41:08Z

No description provided.

lpsinger

Can we have a single description for each field, like this?

"ra_uncertainty": {
  "description": "fill me in",
  "oneOf": [
    {"type": "number"},
    {"type" "array", "maxItems": 2, "items": {"type": "number"}}
  ]
}

lpsinger · 2023-12-30T02:57:44Z

gcn/notices/core/Localization.schema.json

-      "type": "array",
-      "maxItems": 2,
-      "description": "Uncertainty in RA [deg] with optional asymmetric errors, measured from the RA in the schema; respective ex: [r], [r1], [l1-, l1+]. For ellipse, ra axis is r1 or semi-major axis."
+      "description": "Uncertainty in RA [deg], measured w.r.t. RA. A single number for circle, and ellipse, RA axis alligns with semi-major axis. For rectangle, use optional aymmetric errors array, [l1-, l2+].",


What does "Uncertainty in RA measured with respect to RA" mean?

So there have been confusion, about it's understanding. It's uncertainty error measured from RA centre like 2 (-+0.2) or, shape coordinates 2 (1.8, 2.2), so tried to be more explicit, please help with better wordings.

In that case, the name of this field is wrong. It is the uncertainty in the semimajor axis.

But but if it's rectangle?

Vidushi-GitHub · 2024-01-04T18:35:28Z

gcn/notices/core/Localization.schema.json

-      "type": "array",
-      "maxItems": 2,
-      "description": "Uncertainty in RA [deg] with optional asymmetric errors, measured from the RA in the schema; respective ex: [r], [r1], [l1-, l1+]. For ellipse, ra axis is r1 or semi-major axis."
+      "description": "Uncertainty in RA [deg], measured w.r.t. RA. A single number for circle, and ellipse, RA axis alligns with semi-major axis. For rectangle, use optional aymmetric errors array, [l1-, l2+].",


Suggested change

"description": "Uncertainty in RA [deg], measured w.r.t. RA. A single number for circle, and ellipse, RA axis alligns with semi-major axis. For rectangle, use optional aymmetric errors array, [l1-, l2+].",

"description": "Uncertainty in RA [deg]. A single number for circle, and ellipse, RA axis alligns with semi-major axis. For rectangle, use optional aymmetric errors array, [l1-, l2+].",

Vidushi-GitHub · 2024-01-04T18:36:05Z

gcn/notices/core/Localization.schema.json

-      "type": "array",
-      "maxItems": 2,
-      "description": "Uncertainty in Dec [deg] with optional asymmetric errors, measured from the Dec in the schema; respective ex: [r2], [l2-, l2+]. Not reported if circular and Dec axis is r2 or semi-minor axis for ellipse."
+      "description": "Uncertainty in Dec [deg], measured w.r.t. Dec, not reported for circle. A single number for ellipse, Dec axis alligns with semi-minor axis. For rectangle, use optional aymmetric errors array, [b1-, b2+].",


Suggested change

"description": "Uncertainty in Dec [deg], measured w.r.t. Dec, not reported for circle. A single number for ellipse, Dec axis alligns with semi-minor axis. For rectangle, use optional aymmetric errors array, [b1-, b2+].",

"description": "Uncertainty in Dec [deg], not reported for circle. A single number for ellipse, Dec axis alligns with semi-minor axis. For rectangle, use optional aymmetric errors array, [b1-, b2+].",

lpsinger · 2024-01-04T20:23:45Z

I spoke with @jracusin about this. I think that the positional uncertainty fields are poorly conceived and named and need a redesign. What we call ra_uncertainty and dec_uncertainty are really the semi major and semi minor axes of an ellipse containing a given amount of probability (which I believe we left unspecified, another problem).

Please study how these fields are named and encoded in other prominent software applications in astronomy, and propose names and descriptions that are as consistent as possible with prior work:

VOEvent
SExtractor, Scamp, Swarm
IRAF

lpsinger · 2024-01-16T18:47:53Z

@Vidushi-GitHub, please post an update with your progress on this.

Vidushi-GitHub · 2024-01-18T21:51:49Z

Yes, had a discussion with @jracusin. She agreed to correct the description. Vera Rubin uses uncertainty word. Elliptical uncertainty ex: https://gcn.nasa.gov/circulars/35535. We can't shift the center, to make errors symmetric, as mean can be different than negative/positive bars, ex bayesian posterior cases.

lpsinger · 2024-01-18T22:21:50Z

This change is not an improvement over the current situation. There should be two positional uncertainty fields for uncertainty in orthogonal directions and one position angle field. All three fields should be scalar.

You give two reasons for having array-valued positional uncertainties, but I find neither convincing:

Representation of IPN localizations. IPN localizations are either annuli (in the case of two satellites), parallelograms (three spacecraft), or irregular polygons (four or more spacecraft). They are not rectangles, and they cannot be described using the parametrization here. We should not define a vocabulary to define arbitrarily complex region geometry.
Representation of localizations that are not centrally concentrated. Without a quantitative definition of the moments of the distribution in terms of the uncertainty parameters, this is too imprecise to be useful.

I insist that the only localization parametrization that we should support (other than a freeform HEALPix map) is a bounding ellipse. Based on prior astronomy software that reports position uncertainty on points, I strongly recommend giving scalar three parameters: the length of the semi major axis, the length of the semi minor axis, and position angle of an ellipse containing N% probability. If the semi minor axis is omitted, then it is assumed to equal the semi major axis. If the position angle is omitted, then it is assumed to be zero.

Vidushi-GitHub · 2024-01-22T15:57:43Z

What about following, with no min change wrt previous and better description? I am still trying more options. Validation is failing w/t array, so for trials copy-pasted here.

"uncertainty_shape": {
"enum": ["circle", "ellipse"],
"description": "Shape of the uncertainty region for RA and Dec errors. Choose between 'circle' and 'ellipse'.",
"default": "circle"
},
"ra_uncertainty": {
"description": "Uncertainty in RA [degrees]. Provide the radius for 'circle'. For 'ellipse', provide semi-major axes, where RA is assumed to aligned along the semi-major axis.",
"type": "number"
},
"dec_uncertainty": {
"description": "Uncertainty in Dec [degrees]. Do not report for 'circle'. For 'ellipse', provide semi-minor axes, such that semi-minor axis representing the Dec value.",
"type": "number"
},
"position_angle": {
"type": "number",
"description": "Anti-clockwise position angle from the RA axis [degrees]. Used if uncertainty_shape is 'ellipse' and tilted with respect to the standard (RA, Dec) coordinate system.",
"default": 0
},

gcn/notices/core/Localization.schema.json

Co-authored-by: Judy Racusin <[email protected]>

Vidushi-GitHub · 2024-02-20T21:01:04Z

May look redundant, but just for simplicity, I have tried to keep Josh feedback as it is and have added as union. I feel may be in future community start shifting to proposal 2.

gcn/notices/swift/bat/guano.loc_arc_min.example.json

jracusin · 2024-02-20T21:26:17Z

gcn/notices/core/Localization.schema.json

+            "position_error_pa"
+          ]
+        }
+      ]
    },
    "containment_probability": {


If we go with this approach, containment_probability should also be part of error.

jracusin · 2024-02-20T21:28:16Z

gcn/notices/core/Localization.schema.json

-      "type": "number",
-      "description": "Anti-clockwise position angle from RA axis [deg]. Used if uncertainty_shape ellipse or rectangle is tilted w.r.t. standard (RA, Dec) coordinate.",
-      "default": 0
+    "error": {


'error' is a bit too generic, there can be error in many quantities. We could call it position_error. This nesting seems to break the flattening of the schema, and be a bit clunky.

lpsinger

@Vidushi-GitHub, I asked you to make position_error a union of a single number or an array, with the elements of that array being a, b, and pa. This is not an array; it's a composite data structure. None of our other core schema contain composite data types. Let's stick to that convention.

Replace the localization properties `uncertainty_shape`, `ra_uncertainty`, `dec_uncertainty`, and `position_angle` with a single property called `ra_dec_error`. The original localization uncertainty properties were ill-conceived in many ways: 1. The names `ra_uncertainty` and `dec_uncertainty` were misleading. For example, for ellipses, these fields actually represented the lengths of the semi-major and semi-minor axes, and not the uncertainty in ra or dec. 2. The names of these properties were inconsistent with other schema. Every other uncertainty property exept for these has a name ending in `_error`, not `_uncertainty`. 3. The `rectangle` shape was not useful. The most significant examples of quadrilateral localization regions are IPN error boxes. However, these are parallelograms, not rectangles, so the shape would have been under-specified. We have observed that all current missions in GCN either report error circles or HEALPix maps. Given that observation, this patch replaces all of the ill-conceived localization properties with a single `ra_dec_error` field which is the radius in degrees of the error circle. In the future, there might be a mission that requires ellipses. We won't deal with that now, but in that event we would have a few different backwards-compatible options. For example, we could: - Treat the `ra_dec_error` field as the semi-major axis, and add optional fields `ra_dec_error_b` and `ra_dec_error_pa` for the semi-minor axis and the position angle, respectively; or - Allow `ra_dec_error` to be either a single scalar, or an array of up to three numbers, representing the semi-major axis, semi-minor axis, and position angle, respectively. Fixes nasa-gcn#120.

Co-authored-by: Judy Racusin <[email protected]>

gcn/notices/core/Localization.schema.json

Co-authored-by: Leo Singer <[email protected]>

Vidushi-GitHub requested review from lpsinger, dakota002 and jracusin December 29, 2023 01:41

lpsinger requested changes Dec 29, 2023

View reviewed changes

Vidushi-GitHub requested a review from lpsinger December 29, 2023 21:47

lpsinger reviewed Dec 30, 2023

View reviewed changes

Vidushi-GitHub commented Jan 4, 2024

View reviewed changes

Vidushi-GitHub requested a review from lpsinger January 4, 2024 18:36

Vidushi-GitHub added 4 commits January 22, 2024 15:18

either a list or a single number

c86a741

one line description

f48b557

description

2a5b45d

ra, dec error with position angle

6553bf5

Vidushi-GitHub force-pushed the vidushi/location branch from e0f2abc to 6553bf5 Compare January 22, 2024 21:09

Vidushi-GitHub requested review from lpsinger and removed request for lpsinger January 22, 2024 21:11

jracusin reviewed Feb 5, 2024

View reviewed changes

gcn/notices/core/Localization.schema.json Outdated Show resolved Hide resolved

jracusin reviewed Feb 5, 2024

View reviewed changes

gcn/notices/core/Localization.schema.json Outdated Show resolved Hide resolved

lpsinger and others added 3 commits February 5, 2024 23:04

Update gcn/notices/core/Localization.schema.json

022e517

Co-authored-by: Judy Racusin <[email protected]>

Update gcn/notices/core/Localization.schema.json

da319be

Co-authored-by: Judy Racusin <[email protected]>

user friendly localization

4a2a5f0

Vidushi-GitHub requested review from jracusin and lpsinger and removed request for lpsinger February 20, 2024 20:59

jracusin reviewed Feb 20, 2024

View reviewed changes

gcn/notices/swift/bat/guano.loc_arc_min.example.json Outdated Show resolved Hide resolved

jracusin reviewed Feb 20, 2024

View reviewed changes

lpsinger requested changes Feb 20, 2024

View reviewed changes

lpsinger mentioned this pull request Feb 21, 2024

Replace localization uncertainty properties with one error radius #124

Closed

Vidushi-GitHub and others added 2 commits February 22, 2024 14:27

Update gcn/notices/swift/bat/guano.loc_arc_min.example.json

e1f8754

Co-authored-by: Judy Racusin <[email protected]>

option of using number or array

5c7dd20

Vidushi-GitHub requested review from dakota002 and removed request for dakota002 February 22, 2024 20:02

scalar for circle and array for ellipse

6e3fab4

Vidushi-GitHub requested review from jracusin and lpsinger February 22, 2024 20:14

lpsinger requested changes Feb 22, 2024

View reviewed changes

gcn/notices/core/Localization.schema.json Outdated Show resolved Hide resolved

Update gcn/notices/core/Localization.schema.json

18ad741

Co-authored-by: Leo Singer <[email protected]>

Vidushi-GitHub merged commit b33fa02 into nasa-gcn:main Feb 23, 2024
1 check passed

Vidushi-GitHub deleted the vidushi/location branch March 14, 2024 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support a list or a single number in localization #120

Support a list or a single number in localization #120

Vidushi-GitHub commented Dec 29, 2023

lpsinger left a comment

lpsinger Dec 30, 2023

Vidushi-GitHub Dec 30, 2023 •

edited

Loading

lpsinger Dec 31, 2023

Vidushi-GitHub Jan 4, 2024

Vidushi-GitHub Jan 4, 2024

Vidushi-GitHub Jan 4, 2024

lpsinger commented Jan 4, 2024

lpsinger commented Jan 16, 2024

Vidushi-GitHub commented Jan 18, 2024

lpsinger commented Jan 18, 2024

Vidushi-GitHub commented Jan 22, 2024 •

edited

Loading

Vidushi-GitHub commented Feb 20, 2024 •

edited

Loading

jracusin Feb 20, 2024

jracusin Feb 20, 2024

lpsinger left a comment

	"description": "Uncertainty in RA [deg], measured w.r.t. RA. A single number for circle, and ellipse, RA axis alligns with semi-major axis. For rectangle, use optional aymmetric errors array, [l1-, l2+].",
	"description": "Uncertainty in RA [deg]. A single number for circle, and ellipse, RA axis alligns with semi-major axis. For rectangle, use optional aymmetric errors array, [l1-, l2+].",

	"description": "Uncertainty in Dec [deg], measured w.r.t. Dec, not reported for circle. A single number for ellipse, Dec axis alligns with semi-minor axis. For rectangle, use optional aymmetric errors array, [b1-, b2+].",
	"description": "Uncertainty in Dec [deg], not reported for circle. A single number for ellipse, Dec axis alligns with semi-minor axis. For rectangle, use optional aymmetric errors array, [b1-, b2+].",

Support a list or a single number in localization #120

Support a list or a single number in localization #120

Conversation

Vidushi-GitHub commented Dec 29, 2023

lpsinger left a comment

Choose a reason for hiding this comment

lpsinger Dec 30, 2023

Choose a reason for hiding this comment

Vidushi-GitHub Dec 30, 2023 • edited Loading

Choose a reason for hiding this comment

lpsinger Dec 31, 2023

Choose a reason for hiding this comment

Vidushi-GitHub Jan 4, 2024

Choose a reason for hiding this comment

Vidushi-GitHub Jan 4, 2024

Choose a reason for hiding this comment

Vidushi-GitHub Jan 4, 2024

Choose a reason for hiding this comment

lpsinger commented Jan 4, 2024

lpsinger commented Jan 16, 2024

Vidushi-GitHub commented Jan 18, 2024

lpsinger commented Jan 18, 2024

Vidushi-GitHub commented Jan 22, 2024 • edited Loading

Vidushi-GitHub commented Feb 20, 2024 • edited Loading

jracusin Feb 20, 2024

Choose a reason for hiding this comment

jracusin Feb 20, 2024

Choose a reason for hiding this comment

lpsinger left a comment

Choose a reason for hiding this comment

Vidushi-GitHub Dec 30, 2023 •

edited

Loading

Vidushi-GitHub commented Jan 22, 2024 •

edited

Loading

Vidushi-GitHub commented Feb 20, 2024 •

edited

Loading