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

Fix CGGTTS error when the observation interval is 1 second #283

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

vicalloy
Copy link
Contributor

I tried to use rinex-cli to create CGGTTS from RINEX, but it failed(not enough candidates match post-fit criteria). After some debugging, I found that the issue occurs when the observation interval is 1 second.

When the observation interval is 1 second, the line if t >= next_release is called twice within the same track, causing the track to be reset twice. (Note: t == next_release, but sched.next_track_start(*t) does not update next_release.)

I attempted to fix the bug, and the code now works. However, I am not sure if the measurements in the track are correct. Please review the code.

Thank you.

@gwbres
Copy link
Collaborator

gwbres commented Dec 23, 2024

Thank you for your contribution 👍

I just tested it with tutorials/CGGTTS and it works as previously.
I'm inquiring "fast" RINex so we can actually test that (I personnaly have never done that before)

  • we have 2024-DOY=127 with OBS/V3/240506_glacier.gz.
    Apparently the CGGTTS tracker uses RINEx.dominant_sample_rate() as a starting point, which seems reasaonnable.
    But for some reason, this method is in failure on that particular dataset. I will inquire that as well.

I am not surprized the current logic does not work in "extreme" cases. First it has not been tested in such conditions. Secondly, it is too complex and should be improved.

The tracking logic is being improved in the 0.17-rc branch that I hope to merge in the next two weeks, so stay tuned. It will come with a lot of API improvements, simplifications and architecture improvements

@gwbres gwbres merged commit fc20d8c into georust:main Dec 23, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants