Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a global LRU cache to avoid recomputing expensive astronomy data (precession, nutation, geometric positions, moon phases, and rise/transit/set calculations), updates specs for rounding-based cache precision, and adjusts the performance benchmark to use random observer coordinates.
- Introduce
Astronoby::Cachesingleton and integrate it into core calculations. - Update specs to compare rounded outputs matching cached precision.
- Change benchmark to randomize observer location per iteration.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/spec_helper.rb | Require cache support helper |
| spec/astronoby/precession_spec.rb | Adjust expectations to use rounded matrix values |
| spec/astronoby/cache_spec.rb | Add tests for LRU cache behavior |
| spec/astronoby/bodies/*_spec.rb | Update expected positional values for cache rounding |
| lib/astronoby/precession.rb | Wrap matrix computation in cache.fetch |
| lib/astronoby/nutation.rb | Wrap iau2000b angle computation in cache.fetch |
| lib/astronoby/events/rise_transit_set_calculator.rb | Replace per-instance cache with global LRU cache |
| lib/astronoby/events/moon_phases.rb | Cache moon phase results per month |
| lib/astronoby/cache.rb | New LRU cache singleton implementation |
| lib/astronoby/bodies/solar_system_body.rb | Cache geometric ephemeris state lookups |
| lib/astronoby.rb | Require the cache in the main library file |
| benchmarks/performance.rb | Use a random observer each iteration to avoid false hits |
Comments suppressed due to low confidence (2)
spec/astronoby/cache_spec.rb:1
- Tests share a global cache instance without clearing state between examples; this can lead to inter-test dependencies. Consider adding
before(:each) { described_class.instance.clear }to isolate each example.
RSpec.describe Astronoby::Cache do
lib/astronoby/cache.rb:1
- [nitpick] The global LRU cache is a significant new feature. Add documentation (e.g., in the README or a new manual page) explaining how to enable/disable caching and configure cache size and precision.
# frozen_string_literal: true
Merged
|
🥳 so excited about this one! |
rhannequin
added a commit
that referenced
this pull request
Jun 6, 2025
With #186, global cache was introduced to increase the performance of the library on the main features. However, because this caching implementation goes with a small reduction of precision (instants are rounded to increase the chance of hitting it), it felt odd to force the user to use it, and to use it exclusively of the author decided. This introduces a global configuration with a nice API to let the user customise it. ## Usage Currently, the configuration has two purposes: * enabling/disabling caching * set customised rounding values for the features using cache ```rb Astronoby.configuration.cache_enabled = true Astronoby.cache_precision(:geometric, 5) ``` This will enable caching globally and geometric positions will be cached based on a rounding of 0.00001 on the instant. As the instant is stored as a Julian Day, this represents 0.1 seconds: the same geometric position won't be re-computed if its instant falls into 0.1 seconds of a previously computed one. ## Defaults By default, caching is disabled. I want to avoid new users discovering the library and having unexpected results. The default behaviour is to compute everything from scratch constantly, except positions in the RTS calculator during computation. ## Performance With no cache by default, the performance is back to a few commits before, even slightly less performant due to the new added complexity. However, with cache enabled, we're back to interesting performance, with the added bonus of customisable configuration. ```rb require_relative "benchmarks/performance" PerformanceBenchmark.new.run # Astronoby Performance Benchmark # Ruby version : 3.4.3 # Date : 2025-05-26 # # Warming up (2 runs, not measured)... # # Measuring (3 runs, each 10 iterations)... # Run 1/3 complete. # Run 2/3 complete. # Run 3/3 complete. # # RESULTS (averaged over 3 runs, each 10 iterations): # rts_event_on: 22.20 ± 0.06 sec # rts_events_between: 17.23 ± 0.05 sec # twilight_event_on: 23.83 ± 0.30 sec # moon_phases: 22.66 ± 0.06 sec # total: 85.92 ± 0.32 sec ``` ```rb Astronoby.configuration.cache_enabled = true require_relative "benchmarks/performance" PerformanceBenchmark.new.run # Astronoby Performance Benchmark # Ruby version : 3.4.3 # Date : 2025-05-26 # # Warming up (2 runs, not measured)... # # Measuring (3 runs, each 10 iterations)... # Run 1/3 complete. # Run 2/3 complete. # Run 3/3 complete. # # RESULTS (averaged over 3 runs, each 10 iterations): # rts_event_on: 10.23 ± 0.04 sec # rts_events_between: 8.60 ± 0.07 sec # twilight_event_on: 8.18 ± 0.04 sec # moon_phases: 0.06 ± 0.00 sec # total: 27.06 ± 0.06 sec ```
Merged
rhannequin
added a commit
that referenced
this pull request
Sep 1, 2025
## 0.8.0 - 2025-09-01 _If you are upgrading: please see [UPGRADING.md]._ ### Bug fixes * Fix UPGRADING documentation ([#178]) * Fix possible division by zero in RTS ([#185]) ### Features * Introduce performance benchmark ([#183]) * Cache positions in RTS calculator ([#182]) * Internal global LRU cache ([#186]) * Global configuration ([#187]) * Compute the constellation a body is in ([#199]) * Add `#phase_angle` and `#illuminated_fraction` to planets ([#200]) * Apparent magnitude ([#201]) * Add #events_between to TwilightCalculator with better accuracy ([#204]) * `#angular_diameter` on Solar System bodies ([#207]) * Fix constellation boundaries near 24h right ascension ([#209]) ### Improvements * Bump standard from 1.49.0 to 1.50.0 by @dependabot ([#177]) * Bump ephem from 0.3.0 to 0.4.1 by @dependabot ([#181], [#191]) * Bump irb from 1.14.3 to 1.15.2 by @dependabot ([#184]) * Bump rspec from 3.13.0 to 3.13.1 by @dependabot ([#188]) * Bump benchmark from 0.4.0 to 0.4.1 by @dependabot ([#189]) * Bump rake from 13.2.1 to 13.3.0 by @dependabot ([#190]) * Bump matrix from 0.4.2 to 0.4.3 by @dependabot ([#193]) * Update rubyzip requirement from ~> 2.3 to ~> 3.0 by @dependabot ([#194]) * Bump rubyzip from 3.0.0 to 3.0.2 by @dependabot ([#202], [#206]) * Exclude benchmarks from release ([#196]) * `Epoch` refactoring into `JulianDate` ([#197]) * Support Ruby 3.4.4 ([#198]) * Bump actions/checkout from 4 to 5 ([#203]) * Internal documentation ([#205]) ### Backward-incompatible changes * `Epoch` refactoring into `JulianDate` ([#197]) * `#angular_diameter` on Solar System bodies ([#207]) * Rename "epoch" ([#208]) ### Closed issues * Consider adding typical usage/deployment info ([#179]) * Performance degradation in v0.7 ([#180]) **Full Changelog**: v0.7.0...v0.8.0 [#177]: #177 [#178]: #178 [#179]: #179 [#180]: #180 [#181]: #181 [#182]: #182 [#183]: #183 [#184]: #184 [#185]: #185 [#186]: #186 [#187]: #187 [#188]: #188 [#189]: #189 [#190]: #190 [#191]: #191 [#193]: #193 [#194]: #194 [#196]: #196 [#197]: #197 [#198]: #198 [#199]: #199 [#200]: #200 [#201]: #201 [#202]: #202 [#203]: #203 [#204]: #204 [#205]: #205 [#206]: #206 [#207]: #207 [#208]: #208 [#209]: #209 [UPGRADING.md]: https://github.com/rhannequin/astronoby/blob/main/UPGRADING.md
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.
One of the main pain point of Astronoby regarding performance is computing a position, especially apparent and topocentric positions.
In calculators like
RiseTransitSetCalculator, many positions need to be computed in order to find precise times of events.This is costly, but also inefficient as some very close positions need to be computed multiple times.
This introduces an internal global LRU cache to save some of the computed data in order not to have to compute them again later.
For efficiency, some data is cached on a rounded instant, to increase the chance of using the cache, without sacrificing the precision too much.
Here is the list of data that are cached:
iau2000bmodel, rounded on around 14 minutesRiseTransitSetCalculator, rounded on around 0.01 secondsImprovements
I modified the
benchmarks/performance.rbfile to use random observer coordinates so that the script reflects a different observer constantly, to avoid false positive results with caching.Using this updated script on
main:Then, on this branch:
This represents the following performance improvements:
rts_event_on: 40% fasterrts_events_between: 37% fastertwilight_event_on: 56% fastermoon_phases: same time the first time, then 98% fasterNext steps
Ideally I would like this behaviour to be configurable:
To explain this last point, I temporarily changed the cached instant rounded from 0.01 seconds to 1 second. While the computed times are off between 1 and 4 seconds, the performance has been increased by an additional 41%.