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

Faster-unprojected-new #1488

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Faster-unprojected-new #1488

wants to merge 8 commits into from

Conversation

steven-murray
Copy link
Contributor

@steven-murray steven-murray commented Oct 21, 2024

Description

This makes both the setting of the UVWs and the setting of the app coords faster when you're initializing an empty UVData for which the baseline-times are rectangular.

Motivated by running HERA simulations: for redundant groups of the full HERA-350 and 17280 times, these operations were taking ~40min. I haven't timed them yet with the updated code, but tests on smaller sizes show at least 2 orders of magnitude speed up.

Motivation and Context

Types of changes

  • Performance

Checklist:

Performance enhancement:

  • My fix includes a new test that breaks as a result of the bug (if possible).
  • All new and existing tests pass.
  • I have updated the CHANGELOG.

Copy link
Member

@bhazelton bhazelton 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 this, just a couple docstrings need to be updated. Also looks like lots of tests are failing.

src/pyuvdata/utils/phasing.py Outdated Show resolved Hide resolved
src/pyuvdata/uvdata/initializers.py Show resolved Hide resolved
@steven-murray steven-murray force-pushed the faster-unprojected-new branch 2 times, most recently from 76974d9 to fdd963c Compare October 29, 2024 10:11
Base automatically changed from lower-memory-beam-interp to main October 31, 2024 16:11
checking of the created object.
- New keyword `all_times_unique` to `calc_app_coords` that short-cuts the calculation
when it is known that all input times are unique.

Copy link
Member

Choose a reason for hiding this comment

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

This needs to be rebased and then this section needs to be combined with the Added section that's already in main, it's above the changed section.

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

Successfully merging this pull request may close these issues.

2 participants