Skip to content

Extend precession test with a second epoch one second later#4863

Open
IsseiHasegawa wants to merge 2 commits into
Stellarium:masterfrom
IsseiHasegawa:test/precession-add-second-jd
Open

Extend precession test with a second epoch one second later#4863
IsseiHasegawa wants to merge 2 commits into
Stellarium:masterfrom
IsseiHasegawa:test/precession-add-second-jd

Conversation

@IsseiHasegawa
Copy link
Copy Markdown
Contributor

Extend precession test with a second epoch (JD + 1s)

The paper table only gives vectors for one JD; repeat the PQXY vs Capitaine matrix comparison for JD + 1s without duplicating tabulated constants.

Made-with: Cursor

Description

  • Extends TestPrecession::testPrecessionAnglesVondrak() in src/tests/testPrecession.cpp.
  • After the existing checks that use the paper’s tabulated vectors for the reference JD, the same PQXY vs Capitaine construction is repeated for JD + 1 second.
  • No new tabulated constants are added; this only strengthens consistency of the two parametrizations on a nearby epoch.
  • No new dependencies.

Fixes # (none — add issue number if you opened one)

Screenshots (if appropriate):

N/A

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • Housekeeping

How Has This Been Tested?

  • Built the testPrecession target and ran:
    • ctest --test-dir build-test --output-on-failure -R testPrecession
  • Result: testPrecession passed.

Test Configuration:

  • Operating system: macOS (darwin 26.x)
  • Graphics Card: N/A (unit test only)

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

The paper table only gives vectors for one JD; repeat the PQXY vs Capitaine
matrix comparison for JD + 1s without duplicating tabulated constants.

Made-with: Cursor
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2026

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

// according to Fig12 in the paper, a few arcseconds of difference between PQXY and Capitaine parametrisation are allowed.
QVERIFY2((angleCap-angleRef)*3600.0<6.0, QString("Angle between rotation matrices too different!").toUtf8());

//TODO: Add more dates and verify this angle difference is limited to what we can see in Fig.12
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding the same dates plus one second IMO does not qualify to remove the intent of the TODO message. Adding a few centuries might do it, but we have no "official" reference values.

* Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335, USA.
*/

#include <QObject>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These includes were probably used by code now only commented away, and will be needed when taking up development in case any doubts in the precession model appear.

@gzotti
Copy link
Copy Markdown
Member

gzotti commented Apr 8, 2026

TBH I don't see the point here.

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