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

Roundtrip to/from gpst_nanoseconds are not equal. #317

Closed
matte1 opened this issue Jul 17, 2024 · 3 comments · Fixed by #319
Closed

Roundtrip to/from gpst_nanoseconds are not equal. #317

matte1 opened this issue Jul 17, 2024 · 3 comments · Fixed by #319
Assignees
Labels

Comments

@matte1
Copy link

matte1 commented Jul 17, 2024

Bug report

Roundtrip conversion of an epoch does not result in identical values.

Describe the bug

A clear and concise description of what the bug is.

To Reproduce

        let epoch = &Epoch::from_str("2020-11-15 12:34:56.789 TDB").unwrap();
        let epoch_as_nanoseconds = epoch.to_gpst_nanoseconds().unwrap();
        let epoch_from_nanoseconds = Epoch::from_gpst_nanoseconds(epoch_as_nanoseconds);
        assert!(
            epoch == &epoch_from_nanoseconds,
            "{} {}",
            epoch,
            epoch_from_nanoseconds
        );

Results in: 2020-11-15T12:34:56.789000000 TDB 2020-11-15T12:34:05.606254592 GPST

Expected behavior

A roundtrip using the provided functions should result in identical inputs.

Version

We noticed this happening on switching from Anise 0.4.0 from 0.3.2

@gwbres
Copy link
Collaborator

gwbres commented Jul 18, 2024

@ChristopherRabotin

I thought all reciprocal cases were tested, is it not entirely ?

@ChristopherRabotin
Copy link
Member

I am just as surprised as you are. I have had time to look into this since it was reported a few hours ago, just that I could reproduce that bug when running through the same steps via the FromStr trait. I found a 65 nanosecond difference in the use case I tested.

@ChristopherRabotin
Copy link
Member

ChristopherRabotin commented Jul 22, 2024

Thanks for the report, Matt. The crux of the issue is related #229 because the code currently converted the u64 nanoseconds input into a f64 prior to initialization. I'm fixing this right now, and hope to release an updated version at the end of the first week of August. If this timeline is too late for you, let me know, and I can release a patch 4.0.0-alpha before then.

I'd be happy to read your thoughts on the latest comment in #186 which would have prevented this issue entirely.

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

Successfully merging a pull request may close this issue.

3 participants