search

DOI-USGS / ale

Abstraction Layer for Ephemerides (ALE)
Other
13 stars 33 forks source link

tkfram and zzdynrot matrix order issue #552

Open AndrewAnnex opened 1 year ago

AndrewAnnex commented 1 year ago

A SpiceyPy user recently opened an issue (https://github.com/AndrewAnnex/SpiceyPy/issues/473) to report that tkfram was returning a incorrectly transposed rotation matrix. I identified this issue to be due to the use of the direct c converted fortran spice functions and that the zzdynrot was also likely effected, while other similar fortran functions (irfrot etc) were not effected. After looking into this further, both zzdynrot and tkfram were added via a pull request to support ALE in 2019 https://github.com/AndrewAnnex/SpiceyPy/pull/301, so this bug has been lurking in the codebase for a while.

I currently have a pull request https://github.com/AndrewAnnex/SpiceyPy/pull/474 that includes the fixes, but this project should evaluate the degree to which they are impacted by this change.

jlaura commented 1 year ago

The ALE project is impacted by this. The project is going to spin to the current version of spiceypy (prior to this fix being applied). A new release of ALE will be made in September, 2023. That version will not be impacted by this fix.

Kelvinrr commented 1 year ago

SpiceyPy was updated to 6.0 in environment.yml and tests pass, next ALE release (1.0), probably after the ALE sprint a few weeks from now, we will update the recipe on conda-forge as well.

acpaquette commented 12 months ago

@Kelvinrr can this be closed?

Kelvinrr commented 9 months ago

I had this sitting in a local branch and forgot to PR it in 💀

up now: https://github.com/conda-forge/ale-feedstock/pull/58