Document the orbitfit status flags - #495
Conversation
The flag column has eleven values in use and none of them were written down anywhere in the documentation. This lists each value, what sets it, and groups them by what they say about the fit. The grouping matters more than the list: 2 and 6 are fits that converged and were then rejected by a gate, while 1, 3, 4, 5 and 7 never converged at all. The values are an enumeration, not a severity ordering. Also notes that both gates are statistical rather than physical, so flag = 0 means the estimator succeeded rather than that the orbit is plausible. See #493.
|
Why not do a severity ordering? |
|
@Hanno Rein ***@***.***>, what are your thoughts on this, given
your experience with REBOUND?
…On Thu, Aug 27, 2026 at 10:01 AM Meg Schwamb ***@***.***> wrote:
*mschwamb* left a comment (Smithsonian/layup#495)
<#495 (comment)>
Why not do a severity ordering?
—
Reply to this email directly, view it on GitHub
<#495?email_source=notifications&email_token=AA5CVWNGP73Y2JRCV6NVOST5MA5LFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUGAZDIMJRHA2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5440241184>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5CVWOHHHMEYIHLO3QTUZD5MA5LFAVCNFSNUABFKJSXA33TNF2G64TZHM4TCNRSHAYTGMRUHNEXG43VMU5TKMRWHAYTSMZXGUY2C5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AA5CVWIE6OCDDIOOXSV3NNL5MA5LFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUGAZDIMJRHA2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/AA5CVWKZTZB32ODLWTW7OX35MA5LFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUGAZDIMJRHA2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
Matthew J. Holman, PhD
Senior Astrophysicist
Center for Astrophysics | Harvard & Smithsonian
60 Garden Street, MS #51
Cambridge, MA 02138
(617) 496-7775
|
|
I don't feel strongly either way. If we do severity ordering, then we might end up with having to change values later when an additional flag value somewhere in the middle gets introduced. One useful thing might be to label |
|
I like the idea of error or success style |
|
Even with the documentation, I'm still a bit confused. It's not clear from this what the "gates" are. Sometimes the documentation refers to "gates currently enabled", sometimes to "both gates". Especially, what is a "statistical" gate? Related, I don't understand the statement "a fit can converge, pass both [both what?], and still describe an implausible orbit". What is an implausible orbit? |
|
I am still confused after reading the draft documentation. After reading the documentation, I would think flag=0 are the only orbits I should look at in my output from layup orbitfitting. Is that correct? |
|
Wouldn't it make more sense to have separate flags for everything? One for converged/not converged and one for each gate? |
|
@hannorein, are suggesting a bitmask? |
|
I wouldn't overcomplicate it. What about just using multiple variables to report outcomes of the multiple steps? I'm suggesting it because it looks like some of the complexity and confusion comes from logic of how the different outcomes of fits/gates are combined. Rather than trying to explain the logic really well, wouldn't it be easier to just report the flags for the different steps and let the user combine/filter them? It's just a suggestion... I could be miss something here! |
|
@hannorein — you were right. Done in #513: separate columns for @mschwamb — yes, |
#513 replaced the single flag with a summary plus five columns reporting the individual facts, so the eleven-value taxonomy this documented no longer describes the code. Answers the two questions the review raised. Meg's - is flag == 0 the only thing to look at - now has a direct answer at the top: yes for orbits you intend to use, and the other columns say why anything else was rejected. Hanno's objections were to undefined terms; 'gates' is gone, each check is named and says what it tests, and there is no longer a 'statistical gate' or an unexplained 'implausible orbit'. Adds the warning issue #499 now tracks: flag == 2 and flag == 6 under-report, because where a fit stopped takes precedence over why it was rejected. Count rejections on failed_csq and failed_cov instead. failed_physical is documented as reserved and currently always 0, since the check it is for is not applied yet.
Follows #497, which applies the hyperbolic-excess-speed check and adds FLAG_IMPLAUSIBLE_ORBIT. failed_physical is no longer reserved. Says what the check does and does not do: it catches the extreme case, the threshold is deliberately generous because layup is expected to fit real interstellar objects, and it will therefore accept short-arc orbits that are implausible without being impossible. Names the constant and says which way to move it. DEPENDS ON #497 - flag 9 does not exist until that merges.
kjnapier
left a comment
kjnapier
left a comment
There was a problem hiding this comment.
This functions as intended. Good enough for now. A robot can, and will, fix it later if need be.
4 participants
The fit outcome is reported in six columns and none of them were written
down anywhere. This adds a docs page describing each one and when to read it.
The columns are
flag, a summary that is0if and only if the fit convergedand passed every check, plus
converged,stage, and threefailed_*columnscarrying the individual verdicts. A clean fit is zero across all of them.
The page also warns that
flag == 2andflag == 6under-report rejections:where a fit stopped overwrites why it was rejected, so rejections should be
counted on
failed_csqandfailed_cov.Closes #499.
Relevant to #493, but documents only what the code does today.