⚡ Optimize inspire_id_to_coords multiplier calculation#25
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request optimizes the multiplier calculation logic in the inspire_id_to_coords function by replacing an O(N) vapply loop with a vectorized approach using unique and match. The optimization focuses on avoiding redundant recalculations when multiple IDs share the same cellsize resolution.
Changes:
- Replaced iterative multiplier calculation with a vectorized approach that calculates multipliers only once per unique cellsize value
- Added
USE.NAMES = FALSEparameter tovapplyfor consistency - Restructured the code to use
uniqueandmatchfor efficient mapping
Comments suppressed due to low confidence (2)
R/inspire_id_to_coords.R:1
- The same optimization applied to
inspire_id_to_coords.Rshould be applied toinspire_id_format.Rat lines 88-90 (long-to-short conversion) and lines 132-134 (short-to-long conversion). These locations have the identical O(N) pattern that could benefit from the vectorized approach usinguniqueandmatch.
#' @rdname inspire_id_coords
R/inspire_id_to_coords.R:1
- The same optimization applied to
inspire_id_to_coords.Rshould be applied toinspire_id_format.Rat lines 88-90 (long-to-short conversion) and lines 132-134 (short-to-long conversion). These locations have the identical O(N) pattern that could benefit from the vectorized approach usinguniqueandmatch.
#' @rdname inspire_id_coords
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⚡ Optimize inspire_id_to_coords multiplier calculation
💡 What
Replaced the O(N)
vapplyloop for calculating coordinate multipliers with a vectorized approach usinguniqueandmatch.🎯 Why
The original code recalculated the multiplier (power of 10 based on trailing zeros) for every single ID. Since there are typically very few unique resolution values (e.g., '1km', '100m'), calculating the multiplier once per unique resolution and mapping it back avoids hundreds of thousands of redundant function calls.
📊 Measured Improvement
Benchmarks on 100,000 synthetic IDs show:
inspire_id_to_coordsfrom ~0.85s to ~0.68s (~20% total improvement).All tests passed.