IAU-ADES / ADES-Master

ADES implementation based on a master XML file
26 stars 7 forks source link

Bugs in conversion by xmltopsv? #66

Open Pachacoti opened 1 day ago

Pachacoti commented 1 day ago

I notice the following two issues when converting Phaethon's astrometry in XML downloaded from the MPC Explorer to PSV: 1) A good number of observations from stationary stations (i.e., not roving or spacecraft stations) are accompanied by empty entries of the location group elements. However, the latest ADES document explicitly states that the location group elements are used ONLY with roving and spacecraft observations. 2) According to the ADES document, values of shapeOcc should be logical (0 for false or 1 for true). However, the conversion returns strings of True and False, inconsistent with the definition. I wonder whether these discrepancies are due to new changes that have not yet been marked in the document, or they are simply bugs in xmltopsv?

federicaspoto commented 1 day ago

@Pachacoti Could you please attach one or two examples of XML files that you downloaded from the MPC Explorer that had these problems?

Thank you, Federica

Pachacoti commented 1 day ago

Federica,

Thank you for looking into this. Here is the zipped file containing the XML I downloaded from the MPC Explorer and the converted PSV of Phaethon.

Cheers, Man-To

3200_ADES.zip

federicaspoto commented 1 day ago

Hi Man-To,

Thank you for sharing the example. I looked into it and I actually believe that the psv file is correct.

  1. The ADES document specifies the XML format. When converting to PSV, the observations remain within the group and share a single header. If some observations come from satellites or roving observers, the header must also include the location of the stations.
  2. shapeOcc is a boolean and it should write as True or False. We can update the documentation, if that's not clear.

Thanks, Federica

stevechesley commented 1 day ago

Yes, the PSV is correct regarding the location group information. You can see that it is not present in the XML, but it can be blank in the PSV if there are nearby observations in the file that do use those fields.

But, @federicaspoto, the "True"/"False" values for shapeOcc are not ADES compliant, and (not surprisingly) that XML file does not validate. The ADES documentations requirements for shapeOcc, which were established in Feb-2022, state, "Logical (Integer 0=False and 1=True). Assumed True if not present."

This could be changed, but not without pain. I know, for example, that Dave H. is submitting with integer values for this field, and we are coding to the ADES standard here.

federicaspoto commented 1 day ago

@stevechesley I can change the Explorer to write 0 or 1, it is very easy. I believe we can close this issue then.

stevechesley commented 1 day ago

OK, should Man-To sumbit a Jira ticket to MPC?

federicaspoto commented 23 hours ago

@stevechesley yes, that sounds good. I'll take care of it. Thank you.

Pachacoti commented 14 hours ago

@stevechesley and @federicaspoto I understand that the (non-blank) location elements must be present for roving and spacecraft lines. My question is, why are these lines combined with those from stationary stations that don't need the location elements under the same header? To comply with ADES rules, it seems more appropriate to separate these lines into different groups. Alternatively, the ADES document could be updated to allow blank entries in the location group, since the blank string is not one of the accepted values according to the document. You mean submitting a Jira ticket on shapeOcc?

stevechesley commented 13 hours ago

Refer to the ADES documentation on PSV. "In the extreme case..., every Data Record could be preceded by its own specific Keyword Record." That would actually be compliant, but nobody wants to see that, and there is no requirement restricting blank entries in PSV. Blank fields are neglected when translating to XML. We see the same situation with blank optional fields like mag or rmsRA, but we do not issue a new keyword record every time photometry appears or disappears in a PSV file. Your example file demonstrates this for rmsRA. The same principle applies for sys.

Yes, I thought you might put in a Jira ticket to the MPC for shapeOcc, which is an Explorer problem not an ADES-Master problem. But I gather that @federicaspoto has taken care of it.

I think you can close this issue now unless you think it is not resolved.

Pachacoti commented 8 hours ago

I see what you mean here. I completely agree with you that it is acceptable for optional fields to be left blank. Yet according to 4.2.2 of this document (https://github.com/IAU-ADES/ADES-Master/blob/master/ADES_Description.pdf): The Location Group must be present for such cases, but must not be present if stn is associated with a stationary MPC observatory code. Wouldn't empty entries still count as "present" even though they are empty, thereby conflicting the quoted statement?

I've submitted a Jira ticket on shapeOcc.