Skip to content

feat: scenario comparison dashboard#4265

Open
Brendan-Lawton wants to merge 3 commits intomainfrom
scenarioComparison_v2
Open

feat: scenario comparison dashboard#4265
Brendan-Lawton wants to merge 3 commits intomainfrom
scenarioComparison_v2

Conversation

@Brendan-Lawton
Copy link
Contributor

I had one issue with this dashboard, which had something to do with the fact that the trips and emissions dashboards sometimes weren't executed before the scenario comparison dashboard. This led to the dashboard just saying "no trips/emissions data". I changed the priorities of those dashboards and it now seems to work as expected.

…. Fully functioning dashboard and the priority issue of the dashboards seems to be fixed.
Copy link
Contributor

@rakow rakow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to not modify the Dashboard interface and subsequently all dashboards for something that is only relevant for one new dashboard. Maybe create a new extension of the interface called ComparisonDashboard and add getPathToBaseCase and setPathToBaseCase there

@Brendan-Lawton
Copy link
Contributor Author

Hi @rakow, ok, thank you for the suggestion! I think the idea is also to be able to generate difference plots for all dashboards if the pathToBaseCase is there, but that will be implemented at a later date. I'll go ahead and make a new interface.

@Brendan-Lawton
Copy link
Contributor Author

I made the changes @rakow! However, unfortunately the trips dashboard class doesn't seem to always run before the scenario comparison dashboard. It showed no trips data once when I ran my test. The other times it successfully ran before the comparison dashboard, so I'm not sure how to force this behavior other than changing the priorities, which I already did.

@rakow
Copy link
Contributor

rakow commented Sep 10, 2025

The priority defines the order of the dashboards in simwrapper. Dashboards don't run any processing code. This should be done in the corresponding analysis class. There you can define that one analysis depends on the output of another one. E.g check ImpactAnalysis for an example.

@Brendan-Lawton
Copy link
Contributor Author

Hey @rakow, setting the analysis class as a dependency worked. Thanks! However, it appears to only work if there is one dependency class set. I currently need the TripAnalysis and AirPollutionAnalysis classes to run and when I set them as dependencies, whichever one is second, fails. For example, the below code results in the AirPollution class not running:

@CommandSpec(requireRunDirectory = true,
dependsOn = {
@dependency(value = TripAnalysis.class, files = "trip_stats.csv"),
@dependency(value = AirPollutionAnalysis.class, files = "emissions_total.csv")
},

@rakow
Copy link
Contributor

rakow commented Sep 11, 2025

Have you tried setting required=true? If it is still not working I need to take a further look next week. Please prepare a test that reproduces the error.

@Brendan-Lawton
Copy link
Contributor Author

I tried that as well, but it unfortunately didn't work. Thanks for taking a look at this next week. You can test this behavior using this test class: ScenarioComparisonAnalysis.java

@rakow
Copy link
Contributor

rakow commented Sep 30, 2025

The ScenarioComparisonDashboardTest is not producing any errors for me.

@tschlenther
Copy link
Contributor

The ScenarioComparisonDashboardTest is not producing any errors for me.

shall we go ahead and merge, then? and treat potential upcoming problems later in other PRs?

@rakow
Copy link
Contributor

rakow commented Oct 2, 2025

No, even if there is no error, I don't know if the result is as expected. The PR also needs some cleanup before it should be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants