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

Tile_qa_plot: get_expids_efftimes(): speed-up, with only reading spectra files #1954

Merged
merged 1 commit into from Jan 5, 2023

Conversation

araichoor
Copy link
Contributor

[re-opening a PR requesting to merge in "daily" this time]

This PR addresses #1951.

It is a speed-up in the desispec.tile_qa_plot.get_expids_efftimes() function.
Instead of opening both all the spectra and the cframe files, this PR only opens the spectra files.

The output should be identical.
I verified this with running the function on ~100 random exposures of the himalayas redux.

For the problematic case (TILEID=80617 in himalayas), it reduces the computation time by 50% (or more, it depends).

@araichoor araichoor changed the title get_expids_efftimes(): speed-up, with only reading spectra files Tile_qa_plot: get_expids_efftimes(): speed-up, with only reading spectra files Jan 4, 2023
@sbailey
Copy link
Contributor

sbailey commented Jan 5, 2023

Thanks. The code looks good. I'll trust your reproducibility testing. The unreleated github unit test failures are due to some new incompatibility with scipy 1.10.x; in PR #1950 Anthony fixed this by pinning scipy==1.7.3 (same as NERSC). To avoid merge conflicts I won't do the same here, but I'll investigate a long term fix to support scipy 1.10.x (it might involve updating desimodel instead of desispec).

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

2 participants