Rjf/state model

[robfitzgerald]

This PR adds a StateModel to Compass which manages the creation and inspection of search state vectors. along the way,

• traversal model updates were transitioned to mutable updates
• removed RwLock wrappers in SearchApp
• some changes to how summaries look

Closes #134.
Closes #115.

[robfitzgerald]

@nreinicke I am with Sarah for a baby appt., but just wanted to message that I think I omitted unit conversion handling. I forget. I'll try and review it when we get home.

[nreinicke]

just wanted to message that I think I omitted unit conversion handling. I forget. I'll try and review it when we get home.

That sounds good.

I was also wondering if you have an example of a config that is working for this update, when I tried to load one of the tomtom denver configs I get:

[2024-03-08T21:42:54Z ERROR routee_compass] Could not build CompassApp from config file: expected field state for TOML not found
Error: CompassConfigurationError(ExpectedFieldForComponent("state", "TOML"))

I think I just need to add a section to configure the state but not exactly sure what that looks like. l'll also look in the code for more details but thought you might have a ready example.

[robfitzgerald]

Right, I plan to put together a PR on the TomTom side of things with these updates. But you can see an example in the speed_test.toml file in the routee_compass crate that would work for a speed traversal model.

…

On Fri, Mar 8, 2024, 2:49 PM Nicholas Reinicke ***@***.***> wrote: just wanted to message that I think I omitted unit conversion handling. I forget. I'll try and review it when we get home. That sounds good. I was also wondering if you have an example of a config that is working for this update, when I tried to load one of the tomtom denver configs I get: [2024-03-08T21:42:54Z ERROR routee_compass] Could not build CompassApp from config file: expected field state for TOML not found Error: CompassConfigurationError(ExpectedFieldForComponent("state", "TOML")) I think I just need to add a section to configure the state but not exactly sure what that looks like. l'll also look in the code for more details but thought you might have a ready example. — Reply to this email directly, view it on GitHub <#136 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVNXDQ5PMRN5OJISL6FZMLYXIW5XAVCNFSM6AAAAABENMCCAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWGQ3DONBVGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[nreinicke]

This looks great! I like how it centralizes the state information and makes it more intuitive. I left a few comments that we can discuss but overall this looks really good.

[nreinicke]

Would it make more sense to compute access cost before traversal cost? If we're defining access as the cost to "enter" a link, would it be possible that the result of that would then trigger some behavior in the traversal model? Like, a PHEV is at 0.001% SOC and then takes a turn into a link such that it only uses gasoline for the entirety of the link.

[robfitzgerald]

we do compute access cost before traversal cost, line 73 then line 89 respectively. 😬

[nreinicke]

oh lordy I totally misread that, you're right!

[nreinicke]

This can be a future work idea but I tested out storing 3 state model entries as a vector and then just scanning the vector to find the right entry and it's about 4x faster to get the last entry and 8x faster to get the first entry (over 1M iterations). I wonder if we could either default to storing the entries as a vector that we scan or expand the internal store such that if we have a large number of state variables (maybe >10) than we use a hash map but if we have a small number than we use a vector.

[robfitzgerald]

totally. that's how Scala implements Map. for Map instances with 4 or less elements, they rely on a scan, and don't actually instantiate a hashmap until 5+. that could make sense, and perhaps we pull it off by turning StateModel into:

pub enum StateModel {
  OneFeature {
    name: String,
    entry: StateModelEntry
  },
  TwoFeatures { 
    name1: String,
    name2: String,
    entry1: StateModelEntry,
    entry2: StateModelEntry,
  },
  ...
  NFeatures(HashMap<String, StateModelEntry>),
}

[nreinicke]

yeah that's a good idea, and then getting it would be simple and performant like:

fn get(&self, name: &str) -> StateModelEntry {
 match self {
   StateModel::OneFeature => self.entry,
   StateModel::TwoFeatures => {
    if self.name1 == name {
     self.entry1
    } else if self.name2 == name {
     self.entry2
    } else { // err },
    // and so forth
   }
 }
}

[nreinicke]

I wonder if we could nest the state parameters within a toml section? I think this setup would require us to put the state key at the top of the toml file since if it's not then it will get nested inside another section. For example if we have the config:

[graph]
edge_list_input_file = "../data/tomtom_denver/edges-compass.csv.gz"
vertex_list_input_file = "../data/tomtom_denver/vertices-compass.csv.gz"
verbose = true

state = [
    { distance_unit = "kilometers", initial = 0.0 },
    { time_unit = "minutes", initial = 0.0 },
]

state would get nested under the graph key. Maybe the config could look something like:

[state_model]
state = [
    { distance_unit = "kilometers", initial = 0.0 },
    { time_unit = "minutes", initial = 0.0 },
]

[robfitzgerald]

ya, you're right, and i thought this was awkward too. i like your suggestion and i think it's a simple change.

[robfitzgerald]

@nreinicke GOT IT. (i think)

[nreinicke]

Cool, I'm down to bring these changes in. I built some new issues: #137 and #138 based on our discussions in this PR.