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

Depth first #66

Merged
merged 31 commits into from
Oct 9, 2017
Merged

Depth first #66

merged 31 commits into from
Oct 9, 2017

Conversation

dkirkby
Copy link
Member

@dkirkby dkirkby commented Oct 6, 2017

This PR adds the following features:

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 56.647% when pulling f6bd910 on depth_first into fc4f794 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 56.636% when pulling 37071ad on depth_first into fc4f794 on master.

@dkirkby
Copy link
Member Author

dkirkby commented Oct 6, 2017

I am still working on an issue with surveymovie so this is not ready to merge yet.

@dkirkby
Copy link
Member Author

dkirkby commented Oct 6, 2017

This is ready to merge now.

@dkirkby dkirkby requested a review from sbailey October 6, 2017 23:50
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 56.636% when pulling ebaf5de on depth_first into fc4f794 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 56.595% when pulling 0d51d17 on depth_first into fc4f794 on master.

@sbailey
Copy link
Contributor

sbailey commented Oct 9, 2017

Regarding the progress columns: these are the "covered", "available", and "planned" columns? They are integer days since start of survey (not dates or MJDs)? For the record could you define (again) the difference between "available" and "planned"? i.e. what's the recipe for knowing when to run fiber assignment on what tiles?

Clarifying: whether a tile is "covered" or not depends upon the rules file, correct? Or is it just "all tiles from previous layers that cover this tile have now been observed" regardless of whether the rules file says you should wait for them or not?

Code looks fine to merge, but I'd like to document how to use those progress columns.

@sbailey
Copy link
Contributor

sbailey commented Oct 9, 2017

I now see that you defined when to run fiberassignment in issue #24:

adds a column available to the progress fits file that records the day number (defined by a new desisurvey.utils.day_number() function) on which the fibers for each tile are assigned. With the default monthly fiber-assignment cadence, this will always fall on the day closest to a full moon.

@sbailey sbailey merged commit 8ab3a83 into master Oct 9, 2017
@sbailey sbailey deleted the depth_first branch October 9, 2017 17:14
@dkirkby
Copy link
Member Author

dkirkby commented Oct 9, 2017

Clarifying: whether a tile is "covered" or not depends upon the rules file, correct?

Correct. In particular, this section of config.yaml determines which passes are pre-requisites for each pass (P0-3 = DARK, P4 = GRAY, P5-7 = BRIGHT):

fiber_assignment_order:
    P1: P0
    P2: P0+P1
    P3: P0+P1
    P6: P5
    P7: P5+P6

I decided to treat this as config, rather than hardcoded, since we currently treat P3 differently from other DARK layers. Also, note that:

P1: P0
P2: P0+P1

is subtly different from:

P1: P0
P2: P1

(but I think this only matters at the edges).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants