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

desi_run_night pernight redshift broken; internal_id bookkeeping #2338

Closed
sbailey opened this issue Aug 23, 2024 · 2 comments
Closed

desi_run_night pernight redshift broken; internal_id bookkeeping #2338

sbailey opened this issue Aug 23, 2024 · 2 comments
Assignees

Comments

@sbailey
Copy link
Contributor

sbailey commented Aug 23, 2024

desi_run_night -n 20230612 --proc-obstypes science --tiles 24782 --dry-run-level 1

fails with

INFO:processing.py:1597:submit_redshifts: Submitting joint redshift fits of type pernight for TILEID 24782.
INFO:processing.py:1648:submit_redshifts: Expids: [185028].

Traceback (most recent call last):
  File "/global/common/software/desi/users/sjbailey/desispec2/bin/desi_run_night", line 123, in <module>
    sys.exit(submit_night(**args.__dict__))
  File "/global/common/software/desi/users/sjbailey/desispec2/py/desispec/scripts/submit_night.py", line 511, in submit_night
    = submit_tilenight_and_redshifts(ptable, sciences, calibjobs, internal_id,
  File "/global/common/software/desi/users/sjbailey/desispec2/py/desispec/workflow/processing.py", line 2095, in submit_tilenight_and_redshifts
    ptable, internal_id = submit_redshifts(ptable, sciences, tnight, internal_id,
  File "/global/common/software/desi/users/sjbailey/desispec2/py/desispec/workflow/processing.py", line 1649, in submit_redshifts
    redshift_prow, internal_id = make_redshift_prow(zprows, tnight, descriptor=zsubtype, internal_id=internal_id)
ValueError: too many values to unpack (expected 2)

It looks like make_redshift_prow returns only the redshift_prow, not the internal_id, but it is also taking the internal_id as an input. It looks like on lines 1601-1602,internal_id is incremented outside of make_redshift_prow, while make_joint_prow increments the internal_id inside the function and returns the new one to use. Let's make that consistent across functions.

We don't necessarily need pernight redshifts for kibo, so I'll pass this to @akremin to inspect rather than making a last minute emergency patch. If this fix is easy, we could incorporate it before the final tag to launch kibo this afternoon.

@akremin
Copy link
Member

akremin commented Aug 23, 2024

This was caused by an incomplete refactor in a previous pull request, which slipped through the cracks. I originally planned to shift from the use of make_redshift_prow to make_joint_row, but didn't quite make the transition here. I have reverted back to using make_redshift_prow as was already implemented. That required removing interal_id as an expected output and incrementing it manually. This is solved in PR #2339 .

I will also point out that we should be using desi_proc_night rather than desi_run_night. To reduce confusion I have also removed desi_run_night in the referenced PR. That should no longer be needed since desi_proc_night has successfully replaced it for many months now. I had originally left it so that it could be removed in a single "house cleaning" pull request, but it now appears that won't happen before Kibo and it is useful to remove this confusion from the tagged desispec.

@akremin
Copy link
Member

akremin commented Aug 24, 2024

Resolved with PR #2339

@akremin akremin closed this as completed Aug 24, 2024
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

No branches or pull requests

2 participants