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

Fix bls 3tuple #1507

Merged
merged 2 commits into from
Dec 4, 2024
Merged

Fix bls 3tuple #1507

merged 2 commits into from
Dec 4, 2024

Conversation

steven-murray
Copy link
Contributor

@steven-murray steven-murray commented Dec 3, 2024

Description

Fixes a bug in selecting on UVData objects where if you used bls= with a list of 3-tuples where there is more than one polarization present, it would only select the last one.

Motivation and Context

Fixes #1505

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change (documentation changes only)
  • Version change
  • Build or continuous integration change

Checklist:

Bug fix checklist:

  • 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

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: dab9ed7 Previous: 72dd3bc Ratio
tests/uvdata/test_mwa_corr_fits.py::test_read_mwa 3.9132754759465858 iter/sec (stddev: 0.035221061015886734) 7.997555508312163 iter/sec (stddev: 0.031847779135263966) 2.04
tests/uvdata/test_mwa_corr_fits.py::test_read_mwax 3.394458930320797 iter/sec (stddev: 0.10118858356508509) 7.004348042731254 iter/sec (stddev: 0.013840146951652157) 2.06

This comment was automatically generated by workflow using github-action-benchmark.

Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.93%. Comparing base (24120fc) to head (dab9ed7).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1507   +/-   ##
=======================================
  Coverage   99.93%   99.93%           
=======================================
  Files          63       63           
  Lines       21834    21834           
=======================================
  Hits        21819    21819           
  Misses         15       15           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bhazelton
Copy link
Member

@jsdillon can you check that this fixes the problem you reported in #1505 ?

@jsdillon
Copy link
Contributor

jsdillon commented Dec 3, 2024

@jsdillon can you check that this fixes the problem you reported in #1505 ?

Yes, I can verify that the code in #1505 produces the expected output now:

Loading 4pol using just the length-2 tuple
Found (145, 244, 'ne')
Found (145, 244, 'ne')
Found (145, 244, 'en')
Found (145, 244, 'ne')

Loading 4pol with length-3 typles using bls=
Found (145, 244, 'ne')
Found (145, 244, 'ne')
Found (145, 244, 'en')
Found (145, 244, 'ne')

Loading en and ne using bls=
Found (145, 244, 'en')
Found (145, 244, 'ne')

Loading ee and nn using bls=
Found (145, 244, 'ee')
Found (145, 244, 'nn')

Loading en and ne using polarizations=
Found (145, 244, 'en')
Found (145, 244, 'ne')

@bhazelton bhazelton merged commit 9317875 into main Dec 4, 2024
49 of 54 checks passed
@bhazelton bhazelton deleted the fix_bls_3tuple branch December 4, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants