Skip to content

A fix for pre-1962-Jan-20 observations#1144

Merged
mschwamb merged 5 commits intomainfrom
simulation-geometry
May 17, 2025
Merged

A fix for pre-1962-Jan-20 observations#1144
mschwamb merged 5 commits intomainfrom
simulation-geometry

Conversation

@matthewholman
Copy link
Collaborator

@matthewholman matthewholman commented Apr 25, 2025

Fixes #1145

This adds code to use IAU_EARTH rather than ITRF for observations before 1962-Jan-20. Thank you, Rahil!

Review Checklist for Source Code Changes

  • Does pip install still work?
  • Have you written a unit test for any new functions?
  • Do all the units tests run successfully?
  • Does Sorcha run successfully on a test set of input files/databases?
  • Have you used black on the files you have updated to confirm python programming style guide enforcement?

@mschwamb
Copy link
Collaborator

Not sure if this PR is ready for review or not. Happy to review if it's ready. Note - the linter check is failing so black needs to be run on the files (I can do that if needed).

@matthewholman
Copy link
Collaborator Author

I still need to run comparisons with JPL Horizons and add unit tests.

m = spice.pxform("IAU_EARTH", "J2000", et)
mp = spice.pxform("IAU_EARTH", "J2000", et + delta_et)
mm = spice.pxform("IAU_EARTH", "J2000", et - delta_et)
# m = spice.pxform("ITRF93", "J2000", et)
Copy link
Collaborator

Choose a reason for hiding this comment

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

JOSS review hasn't happened so any new additions will be reviewed - can we remove the commented out rows so we don't get asked to clean up during JOSS review?

@mschwamb mschwamb removed the request for review from bernardinelli April 27, 2025 14:55
@mschwamb
Copy link
Collaborator

mschwamb commented Apr 27, 2025

Okay. Add @bernardinelli back to review to signal when this is ready for review so this is PR not approved prematurely. This branch of sorcha can be pip installed -e so it's not holding up layup development at the moment while those checks are being done and the unit test test is being developed.

@matthewholman
Copy link
Collaborator Author

matthewholman commented Apr 27, 2025 via email

@mschwamb
Copy link
Collaborator

Looks good - thanks @bernardinelli for getting this over then line with the unit test - thanks @matthewholman for starting the fix.

@mschwamb mschwamb merged commit bd9a143 into main May 17, 2025
7 checks passed
@mschwamb mschwamb deleted the simulation-geometry branch May 17, 2025 08:58
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.

can't get Earth observatory positions past ~1960s

3 participants