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

Ensure use of float64 when calling radec2pix #447

Merged
merged 1 commit into from
Dec 30, 2024
Merged

Conversation

gitosaurus
Copy link
Contributor

Change Description

In the function healpix_shim.py.radec2pix, convert ra and dec to 64-bit floating point before attempting to call cdshealpix.lonlat_to_healpix, which raises a TypeError if it receives 32-bit floating point.

Closes astronomy-commons/hats-import#458 .

Have verified this fix both with an updated unit test, and also by exercising the code in a separate build of a margin cache, which produced another instance of the above issue. This time, using a local checkout of this fix, the mapping phase continued past 5% without any instance of the error (which arose much earlier than the 5% mark before).

  • My PR includes a link to the issue that I am addressing

Code Quality

  • I have read the Contribution Guide
  • My code follows the code style of this project
  • My code builds (or compiles) cleanly without any errors or warnings
  • My code contains relevant comments and necessary documentation

Bug Fix Checklist

  • My fix includes a new test that breaks as a result of the bug (if possible)
  • My change includes a breaking change
    • My change includes backwards compatibility and deprecation warnings (if possible)

Copy link

codecov bot commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.24%. Comparing base (171b901) to head (c61ba25).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #447   +/-   ##
=======================================
  Coverage   93.24%   93.24%           
=======================================
  Files          47       47           
  Lines        2012     2012           
=======================================
  Hits         1876     1876           
  Misses        136      136           

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

Copy link

Before [171b901] After [fb03a6c] Ratio Benchmark (Parameter)
18.9±0.3ms 19.2±0.4ms 1.02 benchmarks.MetadataSuite.time_load_partition_info_order6
75.6±0.2ms 76.3±0.4ms 1.01 benchmarks.MetadataSuite.time_load_partition_info_order7
74.8±0.1ms 75.9±0.5ms 1.01 benchmarks.MetadataSuite.time_load_partition_join_info
13.2±0.4ms 13.3±0.3ms 1.01 benchmarks.Suite.time_inner_pixel_alignment
364±3ms 369±1ms 1.01 benchmarks.Suite.time_outer_pixel_alignment
114±0.3ms 113±0.5ms 1 benchmarks.time_test_alignment_even_sky
41.7±0.4ms 41.5±0.5ms 0.99 benchmarks.Suite.time_pixel_tree_creation
1.06±0.04ms 1.03±0.04ms 0.98 benchmarks.time_test_cone_filter_multiple_order

Click here to view all benchmarks.

Copy link
Member

@nevencaplar nevencaplar left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@gitosaurus gitosaurus merged commit e07916b into main Dec 30, 2024
11 checks passed
@gitosaurus gitosaurus deleted the issue/import/458 branch December 30, 2024 21:58
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.

TypeError when converting ZTF Lightcurves from hipscat -> hats
2 participants