Skip to content

Cache positions in RTS calculator#182

Merged
rhannequin merged 1 commit intomainfrom
rts-cache-positions
May 24, 2025
Merged

Cache positions in RTS calculator#182
rhannequin merged 1 commit intomainfrom
rts-cache-positions

Conversation

@rhannequin
Copy link
Owner

@rhannequin rhannequin commented May 19, 2025

When computing the rise, transit and set times of a celestial body, the position of the body could be computed multiple times for the same instant. This is not optimized, especially while computing a position is one of the most expensive operation of the library, as we speak.

This caches the computed position during a single operation (event_on, ...).

Comparison

In order to make sure the change has a real impact on performance, I wrote a simplistic benchmark and followed the "measure twice, cut once" guideline by running it on main and running it on this branch.

Code

ephem = Astronoby::Ephem.load("spec/support/data/inpop19a_2025_excerpt.bsp")
observer = Astronoby::Observer.new(
  latitude: Astronoby::Angle.zero,
  longitude: Astronoby::Angle.zero
)
start_full_time = Time.now
duration_event_on = 0
duration_events_between = 0
10.times do
  [
    Astronoby::Mercury,
    Astronoby::Venus,
    Astronoby::Mars,
    Astronoby::Jupiter,
    Astronoby::Saturn,
    Astronoby::Uranus,
    Astronoby::Neptune,
  ].each do |planet|
    calculator = Astronoby::RiseTransitSetCalculator.new(
      body: planet,
      observer: observer,
      ephem: ephem
    )
    time_start = Time.now
    (Date.new(2025, 5, 1)..Date.new(2025, 6, 1)).to_a.each do |date|
      calculator.event_on(date)
    end
    duration_event_on += Time.now - time_start
    time_start = Time.now
    calculator.events_between(Time.utc(2025, 7, 1), Time.utc(2025, 8, 1))
    duration_events_between += Time.now - time_start
  end
end
end_full_time = Time.now
puts "Duration event_on: #{duration_event_on.round(2)} seconds"
puts "Duration events_between: #{duration_events_between.round(2)} seconds"
puts "Duration full time: #{(end_full_time - start_full_time).round(2)} seconds"

Results

Initial
Duration event_on: 26.56 seconds
Duration events_between: 22.15 seconds
Duration full time: 48.72 seconds
Fixed
Duration event_on: 17.79 seconds
Duration events_between: 13.69 seconds
Duration full time: 31.47 seconds

Conclusion

We can estimate a performance improvement between 33% and 38%.

@rhannequin rhannequin self-assigned this May 19, 2025
@rhannequin rhannequin mentioned this pull request May 19, 2025
Closed
@trevorturk
Copy link

trevorturk commented May 19, 2025

I'm not an expert at all, but I wanted to link to an example of using Ruby's benchmark-ips here: https://github.com/ruby/json/pull/606/files -- I think it's a modern example of how to use that. IIRC it does some "warmup" and makes things a bit easier to compare. Might be worth considering instead of calculating durations manually etc.

@rhannequin
Copy link
Owner Author

rhannequin commented May 20, 2025 •
edited
Loading

@trevorturk Good point, this is exactly what I had in mind in the future but it makes sense to tackle this immediately: #183

By the way I used it for this PR:

Before:

RESULTS (averaged over 3 runs, each 10 iterations):
  rts_event_on:          26.66  ±  0.18 sec
  rts_events_between:     22.47  ±  0.18 sec
  twilight_event_on:     27.46  ±  0.18 sec
  moon_phases:           22.19  ±  0.12 sec
  total:                 98.78  ±  0.66 sec

After:

RESULTS (averaged over 3 runs, each 10 iterations):
  rts_event_on:          17.11  ±  0.06 sec
  rts_events_between:     13.16  ±  0.03 sec
  twilight_event_on:     18.89  ±  0.05 sec
  moon_phases:           22.24  ±  0.06 sec
  total:                 71.40  ±  0.17 sec

Which confirms:

  • -36% for Astronoby::RiseTransitSetCalculator#event_on
  • -41% for Astronoby::RiseTransitSetCalculator#events_between
  • -31% for Astronoby::TwilightCalculator#event_on
  • (no change for Astronoby::Events::MoonPhases)

@trevorturk
Copy link

trevorturk commented May 20, 2025

🎉

@rhannequin rhannequin merged commit c77c6a3 into main May 24, 2025
42 checks passed
@rhannequin rhannequin deleted the rts-cache-positions branch May 24, 2025 16:47
@rhannequin rhannequin mentioned this pull request Jun 10, 2025
Merged
rhannequin added a commit that referenced this pull request Sep 1, 2025
@rhannequin
v0.8.0
9a03262 
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@rhannequin rhannequin

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rhannequin @trevorturk