Add capacity bidding and consumer landed costs#2
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces edge capacity modeling and a capacity bidding system to the network simulator. Key changes include UI enhancements to display capacity and consumer costs, updated data models, and a significant update to the simulation engine to handle shared edge capacities and bid-based routing priorities. Feedback focuses on performance bottlenecks in the simulation logic, specifically the redundant execution of Dijkstra's algorithm within the main simulation loop and the inefficiency of calculating routes per producer-consumer pair rather than per producer. Additionally, there is a recommendation to deduplicate the traffic name ordering logic shared between the simulation engine and the main view model.
| { | ||
| notes.Add("No producer nodes were defined for this traffic type."); | ||
| var nextCandidate = contexts | ||
| .SelectMany(context => BuildCandidateRoutes(context, adjacency, remainingCapacityByEdgeId)) |
There was a problem hiding this comment.
The simulation loop is highly inefficient. It recalculates all possible routes for all traffic types in every iteration of the while loop, only to select the single best candidate and discard the rest. This results in a massive number of redundant Dijkstra calculations. For a network with
| foreach (var producerNodeId in context.Supply.Where(pair => pair.Value > Epsilon).Select(pair => pair.Key)) | ||
| { | ||
| foreach (var consumerNodeId in demand.Where(pair => pair.Value > Epsilon).Select(pair => pair.Key)) | ||
| foreach (var consumerNodeId in context.Demand.Where(pair => pair.Value > Epsilon).Select(pair => pair.Key)) | ||
| { |
There was a problem hiding this comment.
BuildCandidateRoutes iterates over every producer-consumer pair and calls FindBestRoute (Dijkstra) for each. It is much more efficient to run Dijkstra once per producer to find the shortest paths to all consumers in the network simultaneously. This would reduce the complexity from
| } | ||
| } | ||
|
|
||
| private static List<string> GetOrderedTrafficNames(NetworkModel network) |
There was a problem hiding this comment.
The method GetOrderedTrafficNames is duplicated here and in MainWindowViewModel.cs. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this logic should be moved to a shared service or a static utility class.
1 participant
No description provided.