Skip to content

Leisures#262

Merged
FlxPo merged 9 commits intomainfrom
leisures
Feb 26, 2026
Merged

Leisures#262
FlxPo merged 9 commits intomainfrom
leisures

Conversation

@lucas-boh
Copy link
Contributor

#208 (comment)
Je me rends compte que je n'avais pas fait de PR pour cette issue sur les loisirs.

Cette version permet de récupérer correctement la localisation des zones de loisirs, et de donner un poids en fonction du type de loisir. J'ai déjà utilisé ce module pour Dolancourt, et ça fonctionne.

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.42%. Comparing base (c8c3ac3) to head (19b07bd).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #262   +/-   ##
=======================================
  Coverage   70.42%   70.42%           
=======================================
  Files          56       56           
  Lines        2424     2424           
=======================================
  Hits         1707     1707           
  Misses        717      717           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@FlxPo
Copy link
Contributor

FlxPo commented Feb 16, 2026

Merci pour la PR ! Quelques questions et remarques :

  • Il y a un fichier leisure et un fichier leisure_old ?
  • Est ce que tu pourrais éviter de modifier prepare_transport_zones et destination_sequence_sampler, qui ne sont pas liés à ce sujet ?
  • Le build de la documentation ne passe pas, à cause d'une erreur d'encoding de fichier (j'ai déjà eu le même problème sur une autre PR). Il faut ouvrir docs/source/model_steps.md dans un éditeur (vs code par ex) avec l'encoding windows 1252, puis le réenregistrer en utf-8. Tu pourrais corriger ça ?

@lucas-boh
Copy link
Contributor Author

Merci pour ta revue !
Du coup je suis revenu en arrière sur les fichiers destination_sequence_sampler.py et prepare_transport_zones , même si j'ai toujours l'erreur sur les types, un rapport de l'erreur ci-dessous.

L’erreur se produit au pl.concat([steps, steps_anchor]) dans

steps = pl.concat([steps, steps_anchor])

SchemaError: type Int64 is incompatible with expected type Int32
...
File ...\mobility\choice_models\destination_sequence_sampler.py, line 540
    steps = pl.concat([steps, steps_anchor])

La chaîne d'appel :

  • pop_trips.get()
  • PopulationTrips.create_and_get_asset
  • PopulationTrips.run_model
  • DestinationSequenceSampler.spatialize_other_motives
  • DestinationSequenceSampler.spatialize_trip_chains_step

@FlxPo
Copy link
Contributor

FlxPo commented Feb 25, 2026

Super.

  • Tu as toujours des changements sur prepare_transport_zones apparemment ?
  • Pour le bug des types, peux tu m'indiquer ta version de polars ?
  • Est ce que tu arrives à remonter la chaîne pour comprendre d'où vient la différence de types ?
  • Est ce qu'il serait possible que ce soit un dataset en cache (opportunités de loisirs ou autres motifs par exemple ?) ? On devrait peut être ajouter des casts explicites aux moments de création des données pour éviter ce type de problèmes (que tu es le seul à avoir a priori...).

@lucas-boh
Copy link
Contributor Author

J'ai polars 1.35.2, avec pyarrow 22.0.0 et pandas 2.3.3

Le mismatch vient d’une chaîne de types incohérente sur les IDs (from, to):

  • leisure.py crée to en pandas Int64.
  • motive.py recaste ensuite to en Int32.
  • D’autres modules castent aussi from/to en Int32 (state_initializer.py, travel_costs_aggregator.py et destination_sequence_sampler.py)
  • Avec des données/caches en parquet-Arrow (souvent lus en 64 bits), ces casts mixtes Int32/Int64 peuvent casser des joins et les concats.

Du coup peut être imposer un type unique from et to, idéalement Int64, dès la création des données ?

@FlxPo
Copy link
Contributor

FlxPo commented Feb 26, 2026

Bien vu, est ce que tu pourrais tracer tous les from/to dans le code et faire un cast en int32 le plus tôt possible à chaque fois, lors de la création des données (voire du premier chargement) ? Et supprimer tous les casts intermédiaires qui peuvent introduire des mismatchs.

Pas la peine d'aller jusqu'à int64 à mon avis pour stocker ces identifiants de zones (les int32 vont jusqu'à 2e9, probablement suffisant !).

@lucas-boh
Copy link
Contributor Author

Voilà ça devrait être plus cohérent, et j'ai plus d'erreurs chez moi.

Rapport des changements :

  • motives/leisure.py: Int64 -> Int32 sur to
  • choice_models/state_initializer.py: ajout cast home_zone_id en Int32
  • choice_models/destination_sequence_sampler.py: suppression du cast intermédiaire to -> Int32 dans utilities

@FlxPo
Copy link
Contributor

FlxPo commented Feb 26, 2026

Merci ça me semble maintenant très bien, j'ai juste remis en place le cast en Float64 utilisé lors de l'échantillonnage, vu que ce n'était pas lié à notre sujet initial ? Dis moi si j'ai raté qqc.

Je pense que ça peut marcher sans cast si polars fait un cast automatique quand on divise ensuite par un float, mais autant être explicite.

@FlxPo FlxPo self-requested a review February 26, 2026 15:05
@FlxPo FlxPo merged commit e60eb0c into main Feb 26, 2026
4 checks passed
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