mobility-team / mobility

Mobility, an open-source library for mobility modelisation
MIT License
18 stars 11 forks source link

Restructuration globale du code #130

Open FlxPo opened 2 months ago

FlxPo commented 2 months ago

Je crée une nouvelle issue globale pour suivre la restructuration du code qui devrait permettre de répondre aux issues actuellement ouvertes. Je suis reparti de la branche carpool et ait ajouté toutes les modifications des autres branches pour avoir la version la plus à jour du code.

La modification la plus importante à ce stade est la création de groupes de classes par mode de transport:

Cela permet de bien séparer les hypothèses entre modes, et d'avoir un endroit unique où fixer des hypothèses par défaut. Cela permet aussi de créer facilement des modes dérivés, comme le covoiturage (basé sur le mode voiture mais avec une classe CarpoolTravelCosts spécifique), ou un mode "multimodal" qui contient les coûts de tous les modes :

class MultiModalMode(TransportMode):

    def __init__(
        self,
        transport_zones: TransportZones,
        modes: Optional[List[TransportMode]] = None
    ):

        if modes is None:

            car_mode = CarMode(transport_zones)

            modes = [
                WalkMode(transport_zones),
                BicycleMode(transport_zones),
                PublicTransportMode(transport_zones),
                car_mode,
                CarpoolMode(car_mode, CarpoolParameters(number_persons=2)),
                CarpoolMode(car_mode, CarpoolParameters(number_persons=3)),
                CarpoolMode(car_mode, CarpoolParameters(number_persons=4))
            ]

        travel_costs = MultiModalTravelCosts(transport_zones, modes)
        super().__init__(travel_costs, ModeParameters("multimodal"))

Cela devrait aussi faciliter l'application de "leviers", puisqu'il suffira de créer autant de versions de ModeParameters que l'on a de variantes du modèle.

Autre petits changements :

@Mind-the-Cap

Mind-the-Cap commented 2 months ago

Pour l'instant je tente de faire tourner small_run de Mobility Grand Genève, j'ai un premier bug pas très clair qui fait que local_admin_unit_id_to devient identique à local_admin_unit_id_from dans tous les cas, et qui semble venir des travel_costs de covoiturage qui sont ensuite fusionnés grâce à aggregate_travel_costs

FlxPo commented 2 months ago

Peux tu essayer de supprimer le fichier local_admin_units.parquet dans ton dossier "data" ? J'ai ajouté une colonne "country" pour éviter d'avoir à le déduire ensuite de l'id des zones, peut être qu'elle manque ?

Mind-the-Cap commented 2 months ago

Oui, déjà tenté ! Je vais implémenter la colonne, elle me semble manquer et ça pourrait résoudre le problème.

Mind-the-Cap commented 2 months ago

Autre solution car il y avait besoin des admin_ids pour certaines fonctionnalités du covoiturage : corriger le merge (commit 1, commit 2). J'espère que ça ne casse pas autre chose par ailleurs, mais comme ça je n'ai pas l'impression

Mind-the-Cap commented 2 months ago

Toutes les instances de ModeParameters sont bien destinées à être appelées dans le run, n'est-ce pas ?

car_params = mobility.CarParameters(
    cost_of_time_c0_short = 35.82,
    cost_of_time_c0 = 24.86,
    cost_of_time_c1 = 0.5)

par exemple.

Dans ce cas, est-on d'accord qu'il faut les rajouter dans tout un tas de fichiers init ? Est-ce qu'on souhaite les appeler comme mobility.CarParameters ou mobility.transport_modes.CarParameters ?

FlxPo commented 2 months ago

Potentiellement oui, même si l'utilisateur doit pouvoir aussi laisser les valeurs par défaut et ne rien spécifier du tout.

Et oui je serai plutot pour exposer tout ce qui est utile à l'utilisateur au plus haut niveau (mobility.CarParameters), ce qui me semble le plus courant et le plus simple ?