Skip to content

191, third PR#251

Merged
ChristopherRabotin merged 8 commits into
masterfrom
191-ter
Jun 1, 2024
Merged

191, third PR#251
ChristopherRabotin merged 8 commits into
masterfrom
191-ter

Conversation

@ChristopherRabotin

Copy link
Copy Markdown
Member

Summary

All errors derive partial equality, update to hifitime v4 (master branch, so not release ready), add an explicit Cartesian constructor to Orbit.

Architectural Changes

No change

New Features

No change

Improvements

No change

Bug Fixes

No change

Testing and validation

Detail the changes in tests, including new tests and validations

Documentation

This PR does not primarily deal with documentation changes.

@ChristopherRabotin ChristopherRabotin left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look trivial to me.

@ChristopherRabotin

Copy link
Copy Markdown
Member Author

@gwbres, I'm having quite a weird situation here with hifitime v4. It should be more precise when doing time scale conversions. However, in the case of this PR, I seem to need to add rounding of one nanosecond to ensure that ANISE doesn't fail when the conversion from f64 to Duration is rounded to the nearest nanosecond. I'm also actively debugging the validation test where the epoch that SPICE is queried is at:
2000-01-01T12:00:32.183927285, instead of
2000-01-01T12:00:32.183927295

Just letting you know the progress. I think that like most other bugs I encounter, I'm baffled by them for just a few hours and then understand them.

@gwbres

gwbres commented May 31, 2024

Copy link
Copy Markdown
Collaborator

I'm also actively debugging the validation test where the epoch that SPICE is queried is at: 2000-01-01T12:00:32.183927285, instead of 2000-01-01T12:00:32.183927295

could you explain what steps led to that 10ns error ?

@ChristopherRabotin

Copy link
Copy Markdown
Member Author

I wonder if some of my changes in the ANISE code didn't lead to that 10 nanosecond error. I've removed those changes now and we'll see if the validation passes. I'm a bit surprised at the behavior to be honest: all of our other tests on v4 should an improvement in the precision, and here we're seeing a degradation in the precision close to J2000 (ref epoch of the ET time scale) but a significant improvement in the precision close to today's date.

@ChristopherRabotin ChristopherRabotin merged commit 2080280 into master Jun 1, 2024
@ChristopherRabotin ChristopherRabotin deleted the 191-ter branch June 1, 2024 05:24
@ChristopherRabotin

Copy link
Copy Markdown
Member Author

@gwbres I cannot reproduce the ten nanosecond error I was seeing. I saw it when trying to query the SPICE file and ANISE for the same epoch at the very start of the ephemeris data, ie at one point only. If it were an issue with hifitime, then first we would have caught it in the hifitime tests, but also, this validation test would have failed: it would have showed a difference in the computed position between ANISE and SPICE. This test does not show this at all.

In fact, more in line with what I expected, the test with v4 show better results! With v3, the time conversion would lead to a small error in exactly ten of the 110k queries of the ephemeris data. With v4, we now see exactly zero errors.

Closes #191

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