Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added fast processing for RINEX 3 #85

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

breid-phys
Copy link

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
Copy link
Member

Thank you this has been a long-requested feature!

@VOMAY
Copy link
Contributor

VOMAY commented Sep 8, 2022

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

This will close: #23

@scivision
Copy link
Member

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

@ielenik
Copy link

ielenik commented Feb 8, 2023

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
Copy link
Author

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
Copy link

ielenik commented Feb 10, 2023

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
Copy link

aclel commented Dec 10, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants