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

Add cross night information to zdashboard #2285

Merged
merged 5 commits into from
Jul 1, 2024
Merged

Add cross night information to zdashboard #2285

merged 5 commits into from
Jul 1, 2024

Conversation

akremin
Copy link
Member

@akremin akremin commented Jun 21, 2024

This updates the redshift dashboard to account for cross night information in the processing of a cumulative tile. This is important when on an earlier night e.g. 10 petals were available but on a later night e.g. 8 petals were available. Before the dashboard would show purple "overfull" values for such tiles because it was expecting 8 redrock files from the 8 petals on that night. Now it correctly identifies that there is petal information for all 10 petals available to the cumulative coadd from the previous night and expects 10 redrock files to be generated.

I tested this on iron, jura, and daily. It worked on all three. Iron shows some missing pernight files, which upon further investigation were accidentally run and then deleted. The dashboard correctly identified them in the processing_table but the state of iron is correct because we didn't want those files.

Jura looks great except for one special tile that we were already aware of that had issues being processed.

Daily looks good for the past two month. Months earlier than that indicate occasional issues in generating a single tile-qa or emline file. Both of these are known issues when coadding very old processings of exposures with modern daily exposures. This only occurs in daily.

Testing code is here: /global/cfs/cdirs/desi/users/kremin/PRs/crossnight_zdash

The dashboards are here:
https://data.desi.lbl.gov/desi/users/kremin/PRs/crossnight_zdash/daily_dashboard/zdashboard.html
https://data.desi.lbl.gov/desi/users/kremin/PRs/crossnight_zdash/iron_dashboard/zdashboard.html
https://data.desi.lbl.gov/desi/users/kremin/PRs/crossnight_zdash/jura_dashboard/zdashboard.html

From these three I don't see any issues with the new cross-night camword logic of the updated dashboard code.

@akremin akremin requested a review from sbailey June 21, 2024 22:53
Copy link
Contributor

@sbailey sbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this. I'm trusting the real-world testing of running on various prods for the correctness of this. I put two docstring requests inline, and noted a corner case bug in camword_to_spectros that pre-dates this PR, but I think is also easily fixed (let's also include a unit test for that).

py/desispec/scripts/zprocdashboard.py Show resolved Hide resolved
py/desispec/io/util.py Show resolved Hide resolved
@akremin
Copy link
Member Author

akremin commented Jun 25, 2024

Thank you for the review. I have made the requested changes. The tests are failing for what appears to be an unrelated reason: "ModuleNotFoundError: No module named 'numpy._typing'" in many of the tests.

@sbailey
Copy link
Contributor

sbailey commented Jun 28, 2024

It looks like the tests are using scipy 1.14.0 which was released a few days ago, and that uses numpy._typing which doesn't exist in earlier versions of numpy (but maybe does in numpy 2.x which we aren't using?).

I'm not going to take any merge action on a Friday afternoon, but I'm also considering if we should merge this into "jura" instead of "main". This plus PR #2284 would make the final 0.63.x tag to go with Jura, then we merge that branch into main and proceed with other development.

@sbailey
Copy link
Contributor

sbailey commented Jul 1, 2024

For the record:

  • I constrained scipy<1.14 in requirements.txt to solve the scipy/numpy version incompatibility. We can unpin that in the future when we port ourselves to numpy 2.x and/or the community sorts out the scipy/numpy incompatibility (oddly I didn't find a ticket or other chatter about it).
  • Rather than rebase off of jura and then merge into jura, I'll leave this as-is merging into main. That means the final jura tag won't have this change used to generate the final jura ops dashboards, but I think that is ok.

@sbailey sbailey merged commit ad0575f into main Jul 1, 2024
26 checks passed
@sbailey sbailey deleted the crossnight_zdash branch July 1, 2024 22:02
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.

2 participants