SCHED 407: Time accounting #280
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #280 +/- ##
==========================================
- Coverage 79.74% 79.43% -0.32%
==========================================
Files 64 64
Lines 3960 3978 +18
==========================================
+ Hits 3158 3160 +2
- Misses 802 818 +16
☔ View full report in Codecov by Sentry. |
| # check that Observation is Observed | ||
| observation.status = ObservationStatus.ONGOING |
There was a problem hiding this comment.
A bit confused about this... why are we always setting to ONGOING?
I think we need to check if the sequence is complete and set to OBSERVED if it is, and ONGOING if it isn't, like here:
| atom.program_used = atom.prog_time | ||
| atom.partner_used = atom.part_time |
There was a problem hiding this comment.
I think this is right... we assume they were completed, so we use up all their time.
There was a problem hiding this comment.
This is close but won't work correctly when the observation is split. At the moment the entire sequence is set to OBSERVED and charged, even if it should should be split. If only first n atoms are to be observed, then , then only the first 5 atoms are updated. If the first atom to be observed is not the first one (index 0), then the loop must start in the middle of the sequence.
I recall now, looking at the plan classes, that the Visit class has field for the starting/ending atom indices but they are currently set to the first and last atoms not matter what. This is something that I forgot to update when adding splitting. We need to add atom_start/end_idx variables to the plan.add method call and then update greedymax.output_plans to pass them. Then the loop can go from v.atom_start_idx to v.atom_end_idx.
| if atom_idx == 0: | ||
| if observation.obs_class == ObservationClass.PARTNERCAL: | ||
| atom.program_used += observation.acq_overhead | ||
| elif (observation.obs_class == ObservationClass.SCIENCE or | ||
| observation.obs_class == ObservationClass.PROGCAL): | ||
| atom.program_used += observation.acq_overhead |
There was a problem hiding this comment.
Instead of doing this, you should just be able to move this out of the for loop and do it directly for first_atom = observation.sequence[0].
There was a problem hiding this comment.
The problem is the for has to be changed as well because it would replace the values in the first iteration and if the first atom is skip entirely then now we have to add the first atom.progtime as well. I guess is just matter of personal preference?
There was a problem hiding this comment.
if atom_idx == 0 should be if atom_idx = v.atom_start_idx.
| atom.observed = True | ||
| atom.qa_state = QAState.PASS |
No description provided.