Hi Hank, On 01/04/2017 10:56 AM, Hank Grabowski
wrote:
Sorry it took so long for me to review. I think the OEM writer is a good addition to Orekit. From what I've read it looks like there are a few pitfalls that we can address before releasing Orekit 9.0. When I wrote EphemerisFile I only had parsing in mind so in some cases I don't think it will work correctly for writing. For example the getFrameString() and getTimeScaleString() methods are allowed to return any string or even null. (The idea is to return the exact representation used in the file.) This creates a problem when attempting to write that value. For example reading from a SP3 file and writing to an OEM file would produce a file with "REF_FRAME = IGS08", which is not a valid frame ID according to Appendix 2 of [1]. Or again, OrekitEphemerisFile implements getFrameString() as Frame.getName() so when writing a TOD ephemeris in the OEM format the file would contain something like "REF_FRAME = TOD/2010 simple EOP" instead of just TOD. (This didn't show up in the test cases since the GCRF frame is used and GCRF's name is always "GCRF".) Since it is not practical to rename the Orekit frames to match their OEM names, some kind of mapping is needed to associate Orekit frames with their OEM names. Time scales and center names likely have similar issues. I think we can make the OEM writer conform better to the OEM standard and make it easier for our users to use it. My proposal is to provide a default mapping for the known common cases and provide a mechanism for the user to override the value written to the file. The common cases would be those listed in [1] and already implemented in Orekit. One other issue I've noticed is that the OEM file requires the entire file to be buffered in memory. This can determine the memory usage for some applications. I think we should integrate the OEM writer with OrekitFixedStepHandler to be able to output an ephemeris as it is computed without buffering the whole thing in memory. What do you think? Best Regards, Evan [1] https://public.ccsds.org/Pubs/502x0b2c1.pdf |