Skip to content

Conversation

@jnation3406
Copy link
Member

@jnation3406 jnation3406 commented Jul 9, 2025

…th eccentricity >= 1 to avoid numerical instability in orbits

This adds in the permanent logging of the MovingViolation number, and then prevents the semester-length visibility caching for targets with eccentricity >= 1. Those targets will instead be cached per-request rather than per-target with just their window specified in the request, which should align it with what happens in the Observation Portal better.

This is currently running on the dev network and works with the high eccentricity request that was failing before.

…th eccentricity >= 1 to avoid numerical instability in orbits
@jnation3406 jnation3406 requested review from jchate6 and markBowman July 9, 2025 05:51
Copy link

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

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

Minor change in how we log the warning would be helpful, unless you have a particular reason for not wanting that.

I think I follow the logic and the code makes sense. I don't see any obvious problems.

My only concerns are that if this does still fail for some reason, the user still won't have any idea why. For them it just won't ever schedule and we won't notice unless we go digging for it in the logs. It would be nice if there were some way to get this error (or just the fact that there IS an error) to the user in cases where the scheduler throws an exception like this.

I also understand this is likely outside the scope of this PR.

Copy link
Contributor

@markBowman markBowman left a comment

Choose a reason for hiding this comment

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

LGTM

@jnation3406 jnation3406 requested a review from jchate6 July 9, 2025 21:20
Copy link

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

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

Excited to get this out there!

@jnation3406 jnation3406 merged commit 592c685 into main Jul 9, 2025
8 checks passed
@jnation3406 jnation3406 deleted the fix/high_eccentricity_targets branch July 9, 2025 22:01
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.

4 participants