geospace-code / georinex

Python RINEX 2 / 3 NAV / OBS / sp3 reader & batch convert to HDF5 with C-like speed
MIT License
216 stars 89 forks source link

Added fast processing for RINEX 3 #85

Open breid-phys opened 2 years ago

breid-phys commented 2 years ago

The RINEX 3 code was re-written following the general idea behind the fast RINEX 2 code, loading all of the values into a numpy array before converting to xarray at the end.

The new code is significantly faster than before, and passes all of the automated tests. If the 'fast' flag is false, it should use the same technique as before.

All of the changes should be within rinexobs3().

scivision commented 2 years ago

Thank you this has been a long-requested feature!

VOMAY commented 1 year ago

I tested this today and it works well. I highly recommend to merge this!

This will close: https://github.com/geospace-code/georinex/issues/23

scivision commented 1 year ago

Thanks, I had to fix other bugs first related to xarray changes. Thanks for your changes and endorsement!

ielenik commented 1 year ago

Looks like the line [186] is incorrect isv = [i for i,s in enumerate(svl) if s in gsv]

usually it works - if you have sats [g01,g02,g03] and this sats appears in same order, you will get correct indexes [0,1,2] But if g01 and g03 was visible from beginning but g02 appears just now (under index 123) you will get [0,1,123] instead of correct [0,123,1]. I do not know how elegantly correct this, but using map like {'g02':123,'g01':0,'g03':2} fixes issue

breid-phys commented 1 year ago

Looks like the line [186] is incorrect isv = [i for i,s in enumerate(svl) if s in gsv]

I think you are right, in the first version of the code I had made an indexing mistake. I had to re-write the fast processing again since it wasn't fast enough, it is a lot more elegant now. I don't think the line you are referring to exists in the code anymore, though.

Please let me know if the problem still exists!

ielenik commented 1 year ago

Oh, this definitely possible. I am not a big specialist in git so that is really possible that I took some outdated version. Sorry to bother you

aclel commented 7 months ago

Thanks @breid-phys, this is great. @scivision is there a plan to merge this in?