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

Switch to Nyx version 2.0.0-beta #16

Merged
merged 12 commits into from
Jul 6, 2024

Conversation

ChristopherRabotin
Copy link
Contributor

This change switches the Eclipse computation to 2.0.0-beta, with ANISE as a backend.

The main question I have is whether the user should provide the Almanac for the Solver structure. The Almanac contains the planetary data and solar system data needed for all of the computations. Hence, it may be useful for the user to provide that depending on their use case, even though in theory, everyone's definition of Earth is the same. If all one needs to do is change the gravitational parameter, then that can be done directly with methods on the [Frame])(https://docs.rs/anise/latest/anise/frames/struct.Frame.html) object.

In the case of the eclipse computation, we actually need to provide tri-axial ellipsoid information to define the shape of the object. But worry not! This data is available in the pck08.pca (and pck11.pca) files that are in the ANISE repo or on the Nyx cloud. The MetaFile structure will automatically download the provided URL if the data does not exist in the user's local temporary directory, or if the CRC of that file does not match the version that is downloaded.

@gwbres
Copy link
Contributor

gwbres commented Jun 23, 2024

Awesome, thank you for the effort !

The main question I have is whether the user should provide the Almanac

It is problematic to me to require internet acces on first deployment.
It's okay to require it during build time. But for me this prevents embedded / hardened environments.
It can also make things more complicated for new comers.

Why is this file (or a more simplistic option) not simply shipped with the library ? especially considering they are hosted as metadata in the library already (not a third party portal).

I would recommend moving to a Cargo build script (rust code triggered by compilation), or other options to trigger some scripts at compilation time.

If you have never used a Cargo build script (which I doubt) you can refer to the one contained in the rinex library, which has the benefit of being simple. It provides the description of the navigation radio messages as specificied by the standards. You will find those in the tokio or mavlink frameworks as well, but they are much more complex

The main question I have is whether the user should provide the Almanac for the Solver structure

To me, this is the only final option. Having maybe an optional input overriden by a default solution.
It is the only solution when targetting ultimate precision, using the latest almanac.
We will also means to provide daily Almanac from RINEX/V4 files, so that would be our entry point

For actual behavior and tests, refer to the PR in RINEX

@ChristopherRabotin
Copy link
Contributor Author

Why is this file (or a more simplistic option) not simply shipped with the library ? especially considering they are hosted as metadata in the library already (not a third party portal).
One of the common problems in my industry is checking the results of another software. To support this, most software I use allows users to configure the input files, including the planetary ephemeris data (for example, NASA Goddard and NASA JPL don't exactly agree on what is the mass of the Earth that should be used, and this is configured in a PCK file).

I understand your use case. I'll propose using rust-embed which is how I handled this in the previous versions of Nyx.

ChristopherRabotin and others added 3 commits June 23, 2024 08:33
You must copy add the DE440s.bsp file to the data folder for rinex.
Rinex expects that this file be available in the AstroData structure.
I could not add it myself because Github doesn't allow adding git lfs to public forks
WARNING: The DE440s.bsp file is large. Once nyx-space/anise#262 is fixed, you should truncate this DE file to only contain a few years and a few planetary ephemerides instead of the whole solar system
Signed-off-by: Guillaume W. Bres <[email protected]>
@gwbres
Copy link
Contributor

gwbres commented Jun 26, 2024

I don't have permission to push git-lfs on this fork so you will have to do it yourself.
Although I disagree this is not the correct location for these files, they should come with ANISE

@ChristopherRabotin
Copy link
Contributor Author

Ok! Once ANISE 0.4 is published in an hour (after dinner), I will update this PR so that it uses the embed_ephem crate feature of ANISE. That feature embeds the de440s.bsp and pck11.pca files into the executable such that you don't need to load or maintain external data, or download it from the internet at runtime.

@gwbres
Copy link
Contributor

gwbres commented Jun 27, 2024

Sounds good. I'm upgrading all my gnss toolkit to hifitime=4.0.0-alpha, so my side should not be blocking.
If I understand correctly, the only problem right now is everything but nyx on your side points to hifitime-4.
Don't forget to run a cargo update because we reference to a few branches here and there

Signed-off-by: Guillaume W. Bres <[email protected]>
@ChristopherRabotin
Copy link
Contributor Author

Yes, that's right, Nyx still need to be released, and I'll work on that in the coming days too. By the weekend, all three packages will have a release.

@gwbres
Copy link
Contributor

gwbres commented Jul 3, 2024

Hello, could we get a Nyx branch that uses the same hifitime version as the others ? we do not need a release

@ChristopherRabotin
Copy link
Contributor Author

ChristopherRabotin commented Jul 3, 2024 via email

@gwbres
Copy link
Contributor

gwbres commented Jul 3, 2024

Thank you I did not catch that! ; )

@ChristopherRabotin
Copy link
Contributor Author

You can now use 2.0.0-rc ;-)

Signed-off-by: Guillaume W. Bres <[email protected]>
@gwbres
Copy link
Contributor

gwbres commented Jul 4, 2024

I'm OK with merging these new features, even without nyx-space/anise#269 as a temporary phase. This topic is more of long term and final target than immediate requirement. Once this topic is "resolved" it will naturaly modify the behavior and requirements

Signed-off-by: Guillaume W. Bres <[email protected]>
@gwbres
Copy link
Contributor

gwbres commented Jul 6, 2024

I will merge this very soon, most likely tomorrow I am unavailable right now.
The current state is ok, but I'm unsure the faulty behavior comes from here or the RINEX section.

Either way I'm merging and we will improve later, i am personaly done with not being able to work on my own stuff

gwbres added 3 commits July 6, 2024 14:58
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
@gwbres gwbres merged commit 80414f3 into rtk-rs:main Jul 6, 2024
1 check passed
@ChristopherRabotin ChristopherRabotin deleted the deps/nyx-2.0.0-beta branch July 10, 2024 14:10
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