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

Add processing of DSN sequential range from ODF files and SPICE error handling module #268

Merged
merged 14 commits into from
Dec 12, 2024

Conversation

valeriof7
Copy link

This pull request adds the processing of sequential range from ODF files and the corresponding observation model for the computed observable.

The current PR also includes the first draft of the SPICE error handling module.

valeriof7 and others added 12 commits November 12, 2024 15:54
  - Included necessary headers for TwoWayDopplerObservationModel and OneWayDopplerObservationModel in
    include/tudat/astro/observation_models/dopplerMeasuredFrequencyObservationModel.h
    (To be verified)
  - Corrected file name typo.
  - Corrected function name typo ("Obsevatio" -> "observation").
  - Added dsnNWayRangeObservationSettings function.

Changes:
- modified:   include/tudat/astro/observation_models/dopplerMeasuredFrequencyObservationModel.h
- modified:   include/tudat/astro/observation_models/observationModel.h
- modified:   include/tudat/interface/json/support/keys.h
- modified:   include/tudat/simulation/estimation_setup/createDirectObservationPartials.h
- modified:   include/tudat/simulation/estimation_setup/createDopplerPartials.h
- modified:   include/tudat/simulation/estimation_setup/createNWayRangePartials.h
- renamed:    include/tudat/simulation/estimation_setup/createObsevationBiasPartial.h -> include/tudat/simulation/estimation_setup/createObservationBiasPartial.h
- modified:   include/tudat/simulation/estimation_setup/createObservationModel.h
- modified:   include/tudat/simulation/estimation_setup/createObservationPartials.h
- modified:   include/tudat/simulation/estimation_setup/orbitDeterminationManager.h
- modified:   src/interface/json/estimation/observation.cpp
- modified:   src/interface/json/support/keys.cpp
- modified:   tests/src/interface/json/inputs/unitTestObservation/observationOutput.json
- modified:   tests/src/interface/json/unitTestObservation.cpp
…ationSettings.

Changes:
- modified:   include/tudat/simulation/estimation_setup/createObservationModel.h
…ion_factor'.

Changes:
- modified:   include/tudat/astro/observation_models/dsnNWayRangeObservationModel.h
- modified:   include/tudat/astro/observation_models/lightTimeSolution.h
- modified:   include/tudat/astro/observation_models/observationModel.h
- modified:   src/astro/observation_models/corrections/solarCoronaCorrection.cpp
…n factor

is now calculated using the reference frequency read from odf files. This should be the
uplink frequency.
Add the set_transponder_delay method to the ObservationCollection class.

Changes:
- modified:   include/tudat/astro/observation_models/dsnNWayRangeObservationModel.h
- modified:   include/tudat/astro/observation_models/lightTimeSolution.h
- modified:   include/tudat/astro/observation_models/observationModel.h
- modified:   include/tudat/simulation/estimation_setup/createObservationModel.h
- modified:   include/tudat/simulation/estimation_setup/observations.h
- modified:   include/tudat/simulation/estimation_setup/processOdfFile.h
@DominicDirkx
Copy link
Member

A had a look through the code, and added a few small comments. One question: I don't see an updated unit tests for the DSN n-way range. Is the existing one still good enough with these additions?

I also saw some re-indentation in a bunch of files, this is from the automated formatting that we discussed right (not a problem, but want to make sure I understand)?

@DominicDirkx
Copy link
Member

The CI passes for almost all tests! Only the final check in the unitTestTransmittingFrequencies.cpp file is failing, which is a result of our having adapted how overlapping ramps are dealt with. If you can comment out the final test of this file in your branch and push it, the CI should be automatically re-triggered. When it passes, and the few minor comments I left in the code are resolved (or associated issues opened), I think we can merge this!

@valeriof7
Copy link
Author

Yes the indentation is the auto-formatting. By the way, did you have the time to review that and test it a bit?
The model itself didn't change, something new is just the conversion factor as a function of the uplink band, so I thought we didn't need any additional test, what do you think? Also, do you usually test every method of any class? Specifically, should we write a test for the "set_transponder_delay" method?

@DominicDirkx DominicDirkx merged commit 1e71060 into tudat-team:develop Dec 12, 2024
4 checks passed
@DominicDirkx
Copy link
Member

And merged! I realized I wasn't very clear in what I meant for the transmitter frequency unit tests. I implemented it here:
c5d16a6
re-enabling most of the tests, but removing only the one testing statement that is now failing

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