Ndr/phev #33

nreinicke · 2023-11-16T22:08:28Z

Starts to implement a new PHEV traversal model described in #31.

This uses the simpler approach to deplete the battery first and then switch to gasoline for the remainder of the trip. This also doesn't consider if the vehicle runs out of battery mid-link but rather just gets to the end of the link and then would switch to gasoline on the next link.

@robfitzgerald also mentions in #31:

i dunno, does that really cover what happens in PHEVs? i mean, couldn't the charge sustaining model recharge the battery enough that the PHEV switches back to battery again later in the trip, perhaps multiple times? i think we want to get that right if we're going to model PHEV, otherwise it will miss out on the benefits of regenerative braking on long-distance trips. which, also btw, implies we want to capture regen from the powertrain model, so we want to be sure the models produce those kinds of outputs for downhill segments.

I think it depends on the mode that the PHEV is operating under which varies based on the model of PHEV and their control implementation. In our weekly RouteE meeting, we discussed doing this very simple implementation to start but I agree that it doesn't capture the true nature of the PHEV. In order to get closer to that we would probably have to allow negative electrical energy from the battery model (charge depleting) and the gasoline model (charge sustaining). The battery model will currently return a negative number but I'm capping it to zero since we can't currently handle negative cost. The gasoline model is not returning electrical energy (just gasoline) and so we would need to modify routee-powertrain to allow that.. Once that is implemented we could capture regen braking during both modes and feed it back into the battery.

I also implemented the battery capacity as a config level parameter and not a query level parameter under the premise that it is very vehicle dependent and the user might not know that information.

The query for this mode would look something like this:

{
    "origin_name": "Government Center Station",
    "destination_name": "Cannonball Creek Brewery",
    "origin_x": -105.200146,
    "origin_y": 39.726570,
    "destination_x": -105.234964,
    "destination_y": 39.768477,
    "model_name": "2016_CHEVROLET_Bolt",
    "phev_mode": "true",
    "grid_search": {
        "energy_cost_coefficient": [0.0, 1.0]
    }
}

robfitzgerald · 2023-11-17T15:28:11Z

In our weekly RouteE meeting, we discussed doing this very simple implementation to start

ok, yea, that helps explain how this feature came about, that the group was asking for exactly this. that's what i get for missing a week on sick leave 🤮

as usual i'm trying to think about how we extend the design to more general problems, but i'll try and reel that impulse in a little. but if we make a special case for PHEV on our otherwise generalized energy-based traversal model, we'll have to build around it and then may regret it later when regenerative braking is requested, so i hope you don't mind if i still plan to take a little time with this when i return on monday. i hope that's ok timing for a review.

nreinicke · 2023-11-17T15:47:34Z

as usual i'm trying to think about how we extend the design to more general problems, but i'll try and reel that impulse in a little. but if we make a special case for PHEV on our otherwise generalized energy-based traversal model, we'll have to build around it and then may regret it later when regenerative braking is requested, so i hope you don't mind if i still plan to take a little time with this when i return on monday. i hope that's ok timing for a review.

Oh yeah of course, there wasn't any rush to get this implemented, it's just something we discussed as a possible next step. It will be nice to get this right and try to make it as general as possible.

I think the crux of making this general is that the state representation and energy calculation varies depending on the powertrain type. Maybe we need a new abstraction here to represent the powertrain specific logic? Something like:

pub struct EnergyTraversalModel {
    pub service: Arc<EnergyModelService>,
    pub powertrain: Arc<dyn Powertrain>,
    pub energy_cost_coefficient: f64,
}

impl TraversalModel for EnergyTraversalModel {
    fn initial_state(&self) -> TraversalState {
	self.powertrain.initial_state()
    }

    fn traversal_cost(
        &self,
        _src: &Vertex,
        edge: &Edge,
        _dst: &Vertex,
        state: &TraversalState,
    ) -> Result<TraversalResult, TraversalModelError> {
        let energy = self.powertrain.compute_energy(&edge, &state);

	// mix energy and cost
    }
}

robfitzgerald · 2023-11-20T16:29:55Z

a few questions right off the bat:

your example has a struct EnergyTraversalModel, is this intended to be a replacement for SpeedGradeModel that generalizes the powertrain?
your suggested Powertrain trait has a lot in common with the SpeedGradePredictionModel trait, is it worth merging the two into one, or is it still helpful to separate prediction from the powertrain?
why do we have separate models for charge sustaining and charge depleting?
you mention "The battery model will currently return a negative number but I'm capping it to zero since we can't currently handle negative cost" - but cost isn't the same thing as state, so, why do we need to cap this?

nreinicke · 2023-11-20T16:59:36Z

your example has a struct EnergyTraversalModel, is this intended to be a replacement for SpeedGradeModel that generalizes the powertrain?

yeah that was my idea with an eye towards eventually generalizing other aspects of it like which features it uses (maybe speed, grade, and some other important feature that we haven't found yet).

your suggested Powertrain trait has a lot in common with the SpeedGradePredictionModel trait, is it worth merging the two into one, or is it still helpful to separate prediction from the powertrain?

That's a good question and I'm not totally sure but I think it still makes sense to keep these distinct so we can keep the separation between how the prediction model is represented as code (i.e. smartcore / onnx / another ML library) and the logic of how we use the results from the prediction model (PHEV, EV, etc)

why do we have separate models for charge sustaining and charge depleting?

That's a function of how fastsim represents the models, there are separate models for when the PHEV is operating on battery only and when the PHEV is using both the gasoline and battery together. So then we also need two separate RouteE Powertrain models to capture each of these operating modes and build our own "control logic" for when to apply each one.

You mention "The battery model will currently return a negative number but I'm capping it to zero since we can't currently handle negative cost" - but cost isn't the same thing as state, so, why do we need to cap this?

That's a good point. We could just cap the contribution of the energy to the cost but still allow a negative cost to contribute directly to the state. This still wouldn't help the algorithm find shortest paths that take advantage of regen braking since it wouldn't directly see those benefits in the cost but it would result in a more realistic battery SOC as the vehicle traverses.

nreinicke · 2023-11-22T22:40:53Z

Okay, I've done quite a bit of modification to this and have create a new Vehicle abstraction that implements a predict_energy() method. Now, the energy traversal model just holds onto a vehicle that could have any way of predicting energy (right now just conventional and plugin in hybrid).

This leaves open a few items that I'll convert to issues if/when we roll this in:

create a "battery electric" vehicle in addition to the existing "conventional" and "plug_in_hybrid" that also keeps track of its battery SOC.
expose the conversion between gasoline and electricity to the config
add in a new diesel energy rate unit and update the config for all diesel vehicles
update the internal configs to support this change

robfitzgerald

great. thanks for putting all the care into how we can build these vehicle types into compass, i think you're clearly the expert of us on how to deal with this stuff.

i like the &[T] state representation, that could shave off a bunch of memory during search.

there's a few requests for changes here, but in general i'm real happy to see that this came together without any deep changes.

robfitzgerald · 2023-11-27T16:27:36Z

rust/routee-compass-powertrain/src/routee/energy_traversal_model.rs

-pub struct SpeedGradeModel {
-    pub service: Arc<SpeedGradeModelService>,
-    pub model_record: Arc<SpeedGradePredictionModelRecord>,
+pub struct EnergyTraversalModel {


i like the name change with the new abstraction

robfitzgerald · 2023-11-27T16:31:25Z

rust/routee-compass-powertrain/src/routee/vehicles/default/plug_in_hybrid.rs

+    }
+}
+
+#[cfg(test)]


i'm noticing that these tests don't do equality assertions. is it possible for us to design it so that the result has an exact expected value, and that those values make sense? just something so we feel comfortable that the math is good.

yeah it's tricky since the vehicle model might not return the exact energy each time (especially if we swap it out with a new model) but perhaps we can add some "order of magnitude" test to make sure it's in a reasonable range

got it. well, i'd follow your lead if this is a requirement for this. i think, maybe knowing it's in the right ballpark could give us a little reassurance our units are correct but then again we don't want to get into the business of writing tests here that are really routee-powertrain tests (tests of the underlying model).

robfitzgerald · 2023-11-27T16:31:48Z

rust/routee-compass-powertrain/src/routee/vehicles/vehicle_trait.rs

+
+pub type VehicleState = Vec<StateVar>;
+
+pub struct VehicleEnergyResult {


pull out to it's own file

robfitzgerald · 2023-11-27T16:32:06Z

rust/routee-compass-powertrain/src/routee/prediction/prediction_model.rs

+}
+
+/// A struct to hold the prediction model and associated metadata
+pub struct PredictionModelRecord {


pull out into it's own file

robfitzgerald · 2023-11-27T16:33:29Z

rust/routee-compass/src/app/compass/config/compass_configuration_field.rs

@@ -29,6 +31,8 @@ impl CompassConfigurationField {
            CompassConfigurationField::Plugins => "plugin",
            CompassConfigurationField::InputPlugins => "input_plugins",
            CompassConfigurationField::OutputPlugins => "output_plugins",
+            CompassConfigurationField::ChargeDeplete => "charge_deplete",


should this be "charge_depleting" and "charge_sustaining"? i don't see us use these contracted versions of these terms anywhere else nor have i heard people say them like this but you def know better than i.

Yeah that's a good point, it should be consistent with what we've been using elsewhere, I'll modify

robfitzgerald · 2023-11-27T16:38:41Z

rust/routee-compass/src/app/compass/config/traversal_model/energy_model_vehicle_builders.rs

+    config_json_extension::ConfigJsonExtensions,
+};
+
+pub trait VehicleBuilder {


two thoughts here.

enum?

this looks a lot like an enum. maybe:

pub enum VehicleBuilder { ConventionalVehicleBuilder, PlugInHybridBuilder } impl VehicleBuilder { pub fn build() {} }

names

"conventional" and "plugin hybrid" names.. i was reviewing those implementations, and, what i felt like i was looking at was 1) "a simple vehicle prediction model wrapper" and 2) "a hybrid or plugin hybrid where there are two models that tradeoff based on a fuel level". i dunno, the word "conventional" is somewhat synonymous with combustion vehicles ¹, maybe we can think of a different name since it can be ICE or BEV here, "SinglePowertrainVehicle" or something like that? and maybe the second is "HybridVehicle" because it could be either kind of hybrid?

Footnotes

https://www.lawinsider.com/dictionary/conventional-vehicle ↩

Yeah good point, conventional is misleading since it can be any powertrain type that just reports a single fuel. Something tricky here is that the hybrid models also just return fuel in gasoline and are suitable for this "conventional" model. The "plugin hybrid" is just a special case for a PHEV vehicle. So, we have a weird setup where we need to find a name for the model that represents the set {ICE, BEV, HEV} and the set {PHEV}. Maybe something like "SingleFuelVehicle" and "DualFuelVehicle" that represents the fuel that goes into the vehicle?

Something tricky here is that the hybrid models also just return fuel in gasoline and are suitable for this "conventional" model

but, is that an implementation detail of how we're currently talking about hybrid, or an implementation detail of how FASTSim does it? i thought our long-term goal might be that these vehicles can refuel. if we don't want to re-write everything for that behavior, then, i would assume hybrid would be a special case of the PHEV code where there simply is no refuel behavior (edit: "at stations; and there would be refuel from regenerative braking")

as for names, something like SingleFuelVehicleModel and HybridFuelVehicleModel might be general enough?

it can be any powertrain type that just reports a single fuel

here's another angle i can talk about my confusion here. i think i'm not clear on what the meaning of the Vehicle abstraction is. i thought it was similar to our HIVE Mechatronics, but including the fuel level, so something that represents all the physics of how energy is modeled for a vehicle.

in the abstraction hierarchy, above Vehicle, we have the TraversalModel, which may have different Vehicles. below it, we have the PredictionModel, which may appear as one or two parts of a Vehicle. it feels like we have the right layers here.

so, maybe we need agreement on what Vehicles represent. i thought it was something like (ideally)

how energy is consumed when idling (access costs, eventually)

how energy is consumed when driving (traversal costs)

how a fuel level is modified by those activities (+-)

how fuel is refilled at stations (access behavior, eventually)

in hybrid systems, how fuel source/powertrain switching behavior occurs

Yeah this is a good point that we need to come up with a good definition for what a Vehicle is. My thinking is starting to trend towards treating a Vehicle as something like a representation of a set of specific vehicle models that share operating behavior. So, if we were to follow that definition, maybe we would actually have vehicle models for each of [ICE, BEV, HEV, PHEV] since they each exhibit different behavior with respect to how they operate (specifically in the domain of consuming and gaining energy).

Something that the above leaves out is that there will still be shared behavior between some of the models. To address that, maybe we can just centralize the shared behavior into functions that are shared between the different vehicles.

robfitzgerald · 2023-11-27T16:39:43Z

rust/routee-compass-powertrain/src/routee/vehicles/vehicle_trait.rs

+    pub updated_state: VehicleState,
+}
+
+pub trait Vehicle: Send + Sync {


two thoughts:

names

i don't think we want to pluralize module names. could we maybe make the module "vehicle" and call this trait "VehicleModel" and file "vehicle_model.rs"? or something like that.

documentation

since this is part of the developer API of Compass, we should be sure to add rust docstrings to this trait. let's mention how these methods relate to the methods of TraversalModel.

robfitzgerald · 2023-11-27T16:44:41Z

rust/routee-compass-powertrain/src/routee/vehicles/default/conventional.rs

+        speed: (Speed, SpeedUnit),
+        grade: (Grade, GradeUnit),
+        distance: (Distance, DistanceUnit),
+        state: &[StateVar],


oh interesting, &[T] vs Vec[T] vs Box<[T]>. we have so many of these, millions at runtime, that picking the right one for how we reference state is going to make a big difference. and given that they are (1) a fixed size, 2) immutable, 3) we don't use any special features of Vec, i think it's a real good call to move to a slice, and just a reference of a slice is probably sufficient. perhaps something we can change everywhere soon.

robfitzgerald · 2023-11-27T17:07:18Z

wow, conflicts don't look bad at all!

nreinicke · 2023-11-27T21:18:36Z

@robfitzgerald I've made an attempt to address the comments that you've made here. With respect to the Vehicle abstraction, perhaps we can spin a new issue out of this to address that? (in the meantime I updated the names to SingleFuelVehicle and DualFuelVehicle but I think this merits more discussion).

I also left a few items for future work that I'll put into new issues:

expose the conversion between gasoline and electricity to the config
add in a new diesel energy rate unit and update the config for all diesel vehicles
update the internal configs to support this change

robfitzgerald

cool, exciting to see these more granular vehicle types rolling into the search and to be modeling them with greater realism. thanks for making the big lift!

robfitzgerald · 2023-11-28T16:18:26Z

rust/routee-compass-powertrain/src/routee/vehicle/default/dual_fuel_vehicle.rs

+        .energy_rate_unit
+        .associated_energy_unit();
+
+    if battery_soc_percent > 0.0 {


assume we can just use the battery

i think logic like this would avoid the need to make that assumption:

if soc == 0: energy = 0.0 use_gas() else: energy = predict() # use battery if energy < 0: energy = 0.0 use_gas() else: use_battery()

Yeah that's a good point, I think I'll add this into a new issue to upgrade the logic of the PHEV in general to be more accurate (including doing things like spitting electric and gas on the same link if we cross the battery threshold during that traversal).

nreinicke added 6 commits November 15, 2023 13:15

check in work for starting to add phev model

be1eb12

Merge branch 'main' into ndr/phev

37e3499

checkpoint phev work

3ba96a9

integrate phev model into builder

a7d5d05

add phev models, build out phev test

760aa65

formatting

3c0e829

nreinicke requested a review from robfitzgerald November 16, 2023 22:09

nreinicke mentioned this pull request Nov 20, 2023

PHEV implementation #31

Closed

nreinicke added 10 commits November 22, 2023 11:11

checkpoint work on vehicle refactor

2a956c3

clippy fixes, formatting

4bc1225

add in vehicle builders

e5dee67

fix a few bugs

9709e53

formatting

868f054

clippy fix

d54d132

doc update

e87a2f6

use github checkout v4

35c2a54

try to fix action

77b636f

Merge branch 'main' into ndr/phev

cfde2eb

robfitzgerald requested changes Nov 27, 2023

View reviewed changes

nreinicke added 2 commits November 27, 2023 10:57

merge main into ndr/phev

96102c6

address review comments

d735c73

formatting

258c834

nreinicke requested a review from robfitzgerald November 27, 2023 21:19

robfitzgerald approved these changes Nov 28, 2023

View reviewed changes

nreinicke merged commit 6f6411b into main Nov 28, 2023
5 checks passed

nreinicke deleted the ndr/phev branch November 28, 2023 16:29

nreinicke mentioned this pull request Nov 28, 2023

Vehicle type discussion #49

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ndr/phev #33

Ndr/phev #33

nreinicke commented Nov 16, 2023 •

edited

Loading

robfitzgerald commented Nov 17, 2023

nreinicke commented Nov 17, 2023 •

edited

Loading

robfitzgerald commented Nov 20, 2023

nreinicke commented Nov 20, 2023

nreinicke commented Nov 22, 2023

robfitzgerald left a comment

robfitzgerald Nov 27, 2023

robfitzgerald Nov 27, 2023

nreinicke Nov 27, 2023

robfitzgerald Nov 27, 2023

robfitzgerald Nov 27, 2023

robfitzgerald Nov 27, 2023

robfitzgerald Nov 27, 2023

nreinicke Nov 27, 2023

robfitzgerald Nov 27, 2023

nreinicke Nov 27, 2023

robfitzgerald Nov 27, 2023 •

edited

Loading

robfitzgerald Nov 27, 2023

nreinicke Nov 27, 2023

nreinicke Nov 27, 2023

robfitzgerald Nov 27, 2023

robfitzgerald Nov 27, 2023

robfitzgerald commented Nov 27, 2023

nreinicke commented Nov 27, 2023 •

edited

Loading

robfitzgerald left a comment

robfitzgerald Nov 28, 2023

nreinicke Nov 28, 2023


		pub type VehicleState = Vec<StateVar>;

		pub struct VehicleEnergyResult {

Ndr/phev #33

Ndr/phev #33

Conversation

nreinicke commented Nov 16, 2023 • edited Loading

robfitzgerald commented Nov 17, 2023

nreinicke commented Nov 17, 2023 • edited Loading

robfitzgerald commented Nov 20, 2023

nreinicke commented Nov 20, 2023

nreinicke commented Nov 22, 2023

robfitzgerald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enum?

names

Footnotes

Choose a reason for hiding this comment

robfitzgerald Nov 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

names

documentation

Choose a reason for hiding this comment

robfitzgerald commented Nov 27, 2023

nreinicke commented Nov 27, 2023 • edited Loading

robfitzgerald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nreinicke commented Nov 16, 2023 •

edited

Loading

nreinicke commented Nov 17, 2023 •

edited

Loading

robfitzgerald Nov 27, 2023 •

edited

Loading

nreinicke commented Nov 27, 2023 •

edited

Loading