feat(discrete_mode_choice): detailed utilities writer#4188
feat(discrete_mode_choice): detailed utilities writer#4188Dib-AEK wants to merge 3 commits intomatsim-org:mainfrom
Conversation
Dib-AEK
changed the title
| public static final String MINIMUM_UTILITY = "minimumUtility"; | ||
| public static final String MAXIMUM_UTILITY = "maximumUtility"; | ||
| public static final String CONSIDER_MINIMUM_UTILITY = "considerMinimumUtility"; | ||
| public static final String WRITE_DETAILED_UTILITIES = "writeDetailedUtilities"; |
There was a problem hiding this comment.
Why add this variable here and not in the DiscreteModeChoiceConfigGroup? It should be more general and not only for the MultinomialLogitSelector.
| this.minimumUtility = minimumUtility; | ||
| this.considerMinimumUtility = considerMinimumUtility; | ||
| this.writeDetailedUtilities = writeDetailedUtilities; | ||
| this.personId = (person!=null)? person.getId(): Id.create("unknown", Person.class); |
There was a problem hiding this comment.
Why is this necessary? If the person==null then we have a problem
|
|
||
| // ===== WRITE TO CSV HERE IF REQUESTED IN THE CONFIG ===== | ||
| if (writeDetailedUtilities && UtilityWriter.isWriterInitialized()) { | ||
| UtilityWriter.writeCandidate(personId, tourTrips, filteredCandidates, selection); |
There was a problem hiding this comment.
This will cause a lot of locking during the mode choice, as we want to write things immediately. Would it be better if we stored the utilities and wrote them out at once later? There is probably some tradeoff between memory and speed.
| @Override | ||
| public UtilitySelector createUtilitySelector() { | ||
| public UtilitySelector createUtilitySelector(Person person, List<DiscreteModeChoiceTrip> tourTrips) { | ||
| return new MaximumSelector(); |
There was a problem hiding this comment.
Why are the person and tourTrips not passed to the MaximumSelector? That one should also record utilities if desired. The same for all the other possible selectors: i.e.,RandomSelector
| @Override | ||
| public NestedLogitSelector createUtilitySelector() { | ||
| public NestedLogitSelector createUtilitySelector(Person person, List<DiscreteModeChoiceTrip> tourTrips) { | ||
| return new NestedLogitSelector(structure, minimumUtility, maximumUtility); |
There was a problem hiding this comment.
NestedLogitSelector should also get Person and List<DiscreteModeChoiceTrip> and write out utilities.
sebhoerl
changed the title
sebhoerl
left a comment
sebhoerl
left a comment
There was a problem hiding this comment.
General comment, taking up Milos' comments: It's not really necessary to modify all the UtilitySelector implementations. It would be cleaner to just write a decorator for a UtilitySelector (e.g. WritingUtilitySelector) that takes another UtilitySelector as an argument. Then the custom logic can happen in addCandidate and select and the call is forwarded to the delegate.
In fact, the decorator pattern can already be applied to the factory of the UtilitySelector. So a WritingUtilityFactory can delegate to any other specific implementation.
Also referring to Milos' comment, I think it is better to collect the candidates in each call to addCandidate and then only perform the writing in select.
sebhoerl
left a comment
sebhoerl
left a comment
There was a problem hiding this comment.
General comment, taking up Milos' comments: It's not really necessary to modify all the UtilitySelector implementations. It would be cleaner to just write a decorator for a UtilitySelector (e.g. WritingUtilitySelector) that takes another UtilitySelector as an argument. Then the custom logic can happen in addCandidate and select and the call is forwarded to the delegate.
In fact, the decorator pattern can already be applied to the factory of the UtilitySelector. So a WritingUtilityFactory can delegate to any other specific implementation.
Also referring to Milos' comment, I think it is better to collect the candidates in each call to addCandidate and then only perform the writing in select.
|
And please add a unit test. |
3 participants
Summary
Adds a new boolean config attribute
writeDetailedUtilities(default:false).When enabled, it writes a CSV with all considered mode options and their utilities for each considered tour, and which of the tours is finally selected for the agent plan.
Purpose
Impact
Config Example: