Update time-integrated reaction rate diagram - #561
Conversation
K20shores
left a comment
There was a problem hiding this comment.
I think the graph looks better overall, but the controls are harder to use. Also, we are missing species. We are also not matching the old view, which could be caused by missing species. See the screenshots below, both for the carbon bond mechanism.
Also, when you get to the flow diagram, you have to scroll down the page to see the full graph, and that's true for the entire results page. I wonder if we could make everything more compact so that the default views don't require any scrolling to see the full page
| maxIndex={maxIdx} | ||
| onChange={handleTimeRangeChange} | ||
| /> | ||
| <div className="flex flex-wrap items-start gap-3 p-2 xs:p-3 sm:p-4 w-full rounded-lg bg-gray-50 text-gray-900 mt-2 xs:mt-3 sm:mt-4"> |
There was a problem hiding this comment.
The numeric input boxes are a little more cumbersome than the original sliders. Further, there's no bounds logic. You can enter -9 into the gross production box, and you can enter a time in seconds that is larger than the allowed range.
What if we made each input option have a slider as well as a text box. We could also display the min and max values allowed for each box, based off of the simulation data.
There was a problem hiding this comment.
Thank you for the feedback and comments. Here are my responses to your comments.
-
Species mismatch in Carbon bond mechanism: In your screenshot, it looks like only a single species is chosen, while the below one appears to show all species. Could you try selecting all species from the dropdown?
One thing I changed is that the species are now read from the solver result data, which meansMisn't shown. The musicbox-acom one does showM. -
Scrolling: We could make it compact. A smaller size of mechanism can fit in one page nicely with no scrolling. A larger mechanism, though would suffer from having to fit the dataset into a smaller visual area.
While no-scrolling would be nice to have, considering what this app is designed to support, I think it would be better to prioritize a larger visualization area so the data can be viewed more clearly.
I think we can aim the have plot/flow diagram fit into a single view without scrolling. This doesn't account for the search bar, notes, summary, etc. -
Missing bound logic: Good point, I added a bounds check and a clamping logic. If a user enters values outside the valid range, it defaults to min/max value of the simulation.
-
Slider: I removed the slider bar because they are not very practical to use or for visualization. The values can easily span multiple orders of magnitude. The lower-end values tend to collapse into just a pixel or two.
My general assumption is that if a user is interested in particular range of values, they would probably already know the approximate mim/max range.
I'm open to bringing the slider back, or trying a different form of visualization if we can address this limitation effectivley.
There was a problem hiding this comment.
@boulderdaze the original logic is you grab all reactions that are somehow connected to the species you select, and then you can deselect from there. I think we need to keep that as an option. Maybe a checkbox you can uncheck for this, or maybe a max depth setting (which can be changed) which determines how many nodes out from the selected species the graph goes.
Separately, the species listed in the graph aren't all of the species in the mechanism. For example, ALD2 is in carbon bond 5 but isn't an option in the list of species. I think this could be a good test to add (somehow), for any given mechanism, ensure the available species for the component are the same for the selected mechanism.
Yeah, maybe it's something we do later, but we definitely need to make the area displaying information be prioritized over all of the sidebars/headers which take up a lot of screen space and aren't adding much value, while still leaving enough gaps.
I'm not sure the user would necissarily know the range of values. For time they might, but the values we are representing are accumulated fluxes, which are going to vary species to species and not be so easy to know. I think at least for the gross production (the original term, flux, might actually be more familiar to scientists) it would make sense to keep both options like we had in the original.
Also, related to missing species, some species selection have no effect. See below for Cl and ClO. I would expect Cl to show up when I click on it. We seem to not be showing things that produce something we click on.
You can see that when I select ClO, we see it get transformed into Cl.
The point of the graph is to figure out what produces what from what, and which reactions are most important. Not showing the pathway when clicking on a species makes that difficult. Anyway, I think the graph is still missing links. Look at the original graph for Cl
This PR: