-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better sync of pixel tasks #590
Conversation
…d redshift tasks. Call getready() at the end of the sync method to propagate dependencies. When getting the list of all tasks, use the default chain order rather than dictionary keys. This ensures that the tasks are processed in execution order.
…lso done by getready()
Offline feedback from @sbailey: for this short term fix, if the spectra and redshift files exist at all, then we should assume they are done. This will give us the wrong answer if (for example) we have completed cframes that have not yet been added to spectra and redshifts, and then we run In the case where files were removed, we would still move the state back to a previous one cleanly. The long term solution is obviously to open the fibermaps in each file and see if the cframe contents are complete. |
…s indicates completion.
@sbailey, this should now implement your desired behavior- can you test again on your mini sim? I verified that using one day of a month production that I can add / remove spectra and zbest files and sync sets the expected state. One note- if spectra files are missing but zbest files exist, then it resets the state back to "spectra = ready, redshift = waiting". Presumably if you are creating zbest files outside of the pipeline then you also have spectra files, so this should not be a problem. |
I confirm that it now works for my minitest case. Thanks. Please add some comments in the code about what the 1/2/3/4 magic number states mean. Some of those are mentioned
When new data arrive, do all covered healpix reset back to state 0 because some cframe files are missing, even if others exist and have been processed all the way through redshifts? Answer that with code comments, not just by answering me here. If those states are already clearly documented in comments elsewhere, it's ok if this updating code just references "see x.y.z for explanation of the states". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works; ok to merge, but please add some additional comments explaining 0/1/2/3 states either as part of this PR or as part of a future update.
The issue of comments and documentation is certainly valid, and documenting the states in the healpix_frame table and when those are changed should be done. It is a much bigger issue than just the DataBase.sync() method changed here (see #531). For the record here: completion of new cframes changes pixel state 0 --> 1. Completion of spectra changes 1 --> 2. Completion of redshifts changes 2 --> 3. So no spectra files will be moved from "done" back to "ready" until after a new cframe file that touches that pixel is completed. |
I added more details to #531. Merging this now. |
The DataBase.sync() method scans the filesystem and rebuilds the database state of all tasks. This works fine for all tasks through cframes. For spectra and redshift tasks, the dependencies on cframes are tracked in the healpix_frame table. This PR attempts to reconstruct the state of these tasks (i.e. waiting, ready, done) based on the filesystem and on the current status of the healpix_frame table. The logic here is the desire to do:
And have the state of the spectra and redshift tasks be correct. The sync command now also calls the getready() method.
Additionally there was a bug in the order of tasktypes returned by db.task_types() (used in several places). The tasktype order was not the same as the execution order. This should close #558 and close #563 .