Skip to content

feat(logbook_position_control): réutiliser un buffer mer précalculé entre appels - #16

Closed
JulienLebranchu wants to merge 1 commit into
fix/logbook_position_control-buffer-performancefrom
feat/logbook_position_control-cache-sea-buffer
Closed

JulienLebranchu wants to merge 1 commit into
fix/logbook_position_control-buffer-performancefrom
feat/logbook_position_control-cache-sea-buffer

Conversation

@JulienLebranchu

Copy link
Copy Markdown
Contributor

Closes #15.

Empilée sur #14 (branche de base : fix/logbook_position_control-buffer-performance) — merge/review #14 en premier.

Changement

  • Nouvelle fonction exportée build_sea_buffer(dataframe3, buffer_sea = 925, simplify_tolerance = 50) : extrait la logique de simplification + bufferisation de la mer, déjà présente dans logbook_position_control().
  • Nouveau paramètre sea_buffer = NULL sur logbook_position_control() :
    • NULL (défaut) → comportement identique à aujourd'hui, aucun changement pour les appelants existants (ex. AkadoR n'a rien à modifier).
    • fourni → l'étape de bufferisation mer (la plus coûteuse) est sautée, l'objet précalculé est utilisé tel quel.

Pas de changement cassant : paramètre additionnel, opt-in.

Mesure (détail dans #15)

3 appels consécutifs, même process R, données réelles (1130 activités / 49 marées) :

Appel 1 Appel 2 Appel 3
Sans cache 6.39s 6.49s 6.19s
Avec sea_buffer précalculé (coût payé une fois : 5.96s) 0.54s 0.46s 0.36s

~12-17x plus rapide par appel répété. Pas de gain mémoire net mesuré à cette échelle (pic RSS comparable, ~545-555 Mio dans les deux cas) — #14 suffit déjà à stabiliser la mémoire pour ce volume ; le gain ici est sur le temps d'exécution.

Vérification

  • identical() == TRUE entre un appel sea_buffer = NULL et un appel sea_buffer = build_sea_buffer(...).
  • Doctest existant de logbook_position_control inchangé et passant.
  • Nouveau doctest pour build_sea_buffer (vérifie la structure de sortie).

Usage côté appelant (ex. AkadoR)

# une fois par session, au chargement du référentiel
sea_buffer <- codama::build_sea_buffer(shape_sea)

# à chaque lancement de calcul dans la session
codama::logbook_position_control(dataframe1, dataframe2, shape_sea,
                                  output = "report", sea_buffer = sea_buffer)

…r across calls

Extracts the sea-buffering logic (simplify + terra::buffer, previously
inline) into a new exported build_sea_buffer(dataframe3, buffer_sea,
simplify_tolerance) helper, and adds an optional `sea_buffer` argument
to logbook_position_control(). When NULL (default), behaviour is
unchanged: the sea buffer is computed internally exactly as before.
When provided, the (dominant) buffering step is skipped entirely and
the precomputed sf object is used directly.

Rationale: shape_sea is a session-level referential in callers like
AkadoR, but logbook_position_control() is called every time a user
(re-)runs a check within that same session, recomputing the identical
buffer each time. See #13 for the original investigation.

Targeted measurement (3 consecutive calls, same R process, real data:
1130 activities / 49 trips):

              | call 1 | call 2 | call 3
  no caching  | 6.39s  | 6.49s  | 6.19s
  precomputed | 0.54s  | 0.46s  | 0.36s   (+5.96s paid once upfront)

-> ~12-17x faster per repeated call once the buffer is precomputed.
Peak memory (RSS) was comparable between the two scenarios at this
data scale (~545-555 MiB either way) now that #13's simplify_tolerance
fix is applied in both cases, so the measured gain here is on runtime,
not memory, at this scale.

Output verified identical() between logbook_position_control() called
with sea_buffer = NULL vs sea_buffer = build_sea_buffer(...). Existing
doctest for logbook_position_control unchanged and passing.
@OceaneBouhineauIRD

Copy link
Copy Markdown
Collaborator

Je ferme cette issue, le patch sf::st_simplify() est implémenté directement dans AkadoR.
L'impact sur les données est minime avec dTolerance = 50, seuls 10 positions sont impactés (tous concerne des objets échoués)

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.

2 participants