Skip to content
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

Mtltime timestamp #347

Merged
merged 6 commits into from May 13, 2021
Merged

Mtltime timestamp #347

merged 6 commits into from May 13, 2021

Conversation

araichoor
Copy link
Contributor

This PR now defaults the args.mtltime fba_launch argument to the latest timestamp in the $DESI_SURVEYOPS/mtl-done-tiles.ecsv file for the considered args.program.
If no value is found, we default to the current time.

We currently add +1min to the MTL latest timestamp to account for current discrepancy between mtl-done-tiles.ecsv and the ledgers.
To be seen how to change that once the fix is done in desitarget

Besides, we did a black formatting of fba_launch.

@araichoor
Copy link
Contributor Author

@geordie666 : Aside comment: with checking how the MTL ledgers reading cuts on the isodate, I see that it is now a "<" cut: https://github.com/desihub/desitarget/blob/b86ae8ebf61024e310e5d102723191d5f23d4007/py/desitarget/io.py#L2552-L2557
Would it be possible to make it a "<=" cut ?

@geordie666
Copy link

Would it be possible to make it a "<=" cut ?. It's not easy to switch this behavior and retain backwards-compatibility. I could add a kwarg specifying that the user wants <= rather than <. But, you could also just add a fraction of a second to the isodate that you pass.

@geordie666
Copy link

I'll add the "<=" kwarg when I add the LyA decisions for the full Main Survey MTL loop, though.

@araichoor
Copy link
Contributor Author

thanks for adding a "<=" option in reading the mtl; that simplifies things on the fba_launch side (otherwise, I don t know if there corner case issues with adding a fraction of second to args.mtltime when reading the isodate0.

@geordie666
Copy link

I think this looks OK, now. I'm sure we'll find a few bugs as we work towards the new MTL loop, though!

@araichoor
Copy link
Contributor Author

I pushed a last commit, triggered by a calm re-reading of the function.
I think it should be fine now:

>>> from fiberassign.fba_launch_io import get_program_latest_timestamp
>>> get_program_latest_timestamp("DARK")
'2021-05-12T19:16:48+00:00'
>>> get_program_latest_timestamp("BRIGHT")
'2021-05-12T19:16:38+00:00'
>>> get_program_latest_timestamp("BACKUP")
'2021-05-02T18:44:45+00:00'
>>> get_program_latest_timestamp("bla") is None
True

should we merge?

@sbailey sbailey merged commit ecb0853 into master May 13, 2021
@sbailey sbailey deleted the mtltime_timestamp branch May 13, 2021 18:47
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.

None yet

3 participants