Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tornado improvements to the tornado component and to VolumetricAnalysis #825

Merged
merged 1 commit into from
Oct 25, 2021
Merged

Tornado improvements to the tornado component and to VolumetricAnalysis #825

merged 1 commit into from
Oct 25, 2021

Conversation

tnatt
Copy link
Collaborator

@tnatt tnatt commented Oct 22, 2021

PR which includes improvements to the tornado component and the "tornado" tab in VolumetricAnalysis.

Main changes to the tornado component:

  • Added option of coloring the tornadobars by sensitivity, and possibility to input a color mapping for the sensitivities.
  • Added hoverlabel to the tornadobars
  • Removed realization info in detailed label

Main layout changes to the "tornado" tab in VolumetricAnalysis:

  • Added option of hiding the tornado table to make more space for the tornados
  • Added subplot option to be able to create separate tornados for a selector (e.g. per region/zone). This is a useful feature that should be implemented in the tornado component in the future.
  • Added option of sharing xaxis between the tornados if the scale is relative
  • Replaced mode selector with buttons to be able to store the previous selection for the different pages in the same manner as the "Inplace distribution" tab

image

Also

  • switch to using function from webviz_subsurface._utils for creating "selected realizations" string from list of integers
  • moved all layout for the tornado tab into one file and moved all callbacks related to this tab into single "controller" file
  • removed all app instances from VolumetricAnalysis

Contributor checklist

  • 🎉 This PR closes #ISSUE_NUMBER.
  • 📜 I have broken down my PR into the following tasks:
    • Task 1
    • Task 2
  • 🤖 I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR.
  • 📖 I have considered adding a new entry in CHANGELOG.md, and added it if should be communicated there.

@tnatt tnatt changed the title Tornado improvements Tornado improvements to the 'tornado component and to VolumetricAnalysis` Oct 22, 2021
@tnatt tnatt changed the title Tornado improvements to the 'tornado component and to VolumetricAnalysis` Tornado improvements to the tornado component and to VolumetricAnalysis Oct 22, 2021
@HansKallekleiv HansKallekleiv added the next release 🚢 To be included in next release label Oct 22, 2021
HansKallekleiv
HansKallekleiv requested changes Oct 23, 2021
View reviewed changes
Copy link
Collaborator

@HansKallekleiv HansKallekleiv left a comment

Choose a reason for hiding this comment

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

These are nice improvements to the TornadoPlot :)

  • There is one bug that must be fixed with clickdata/hovermode.
  • The tests are failing. I think it is because the app is not used in the plugin initialization any more. Just remove the app from the tests as well.

webviz_subsurface/plugins/_volumetric_analysis/volumetric_analysis.py Outdated
@@ -137,7 +138,7 @@ class VolumetricAnalysis(WebvizPluginABC):
# pylint: disable=too-many-arguments
def __init__(
self,
app: Dash,
# app: Dash,
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove

webviz_subsurface/plugins/_volumetric_analysis/views/tornado_view.py Outdated
options=[{"label": i, "value": i} for i in volumemodel.responses],
value=volumemodel.responses[0],
),
html.Details(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be simplified as the wcc.Select now incorporates html.Details directly.
equinor/webviz-core-components#182

webviz_subsurface/plugins/_volumetric_analysis/utils/table_and_figure_utils.py
@@ -133,3 +134,15 @@ def add_correlation_line(figure: go.Figure, xy_min: float, xy_max: float) -> go.
y1=xy_max,
line=dict(color="black", width=2, dash="dash"),
)


def create_figure_matrix(figures: List[go.Figure]) -> List[List[go.Figure]]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider generalizing and move to webviz_subsurface/utils.
Make an issue on it for now?

webviz_subsurface/plugins/_volumetric_analysis/controllers/tornado_controllers.py
for prop in ["options", "value", "disabled"]
)

def sens_colors() -> dict:
Copy link
Collaborator

Choose a reason for hiding this comment

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

These colors are hard-coded here because the Equinor theme does not provide appropriate colors for this use case.

Current theme colors give:
image

Colors provided here:
image

Perhaps we should add a categorical colorscale to the equinor theme?
@asnyv , @anders-kiaer

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can do that, but I didn't get the use case for adding categorical colorscales in addition to the existing colorway/qualitative. Could you expand on the use case (and how it is related to the screenshots above) @HansKallekleiv?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was a bit quick! The current colorway / qualitative setup in the theme is fine. I'm struggling a bit with the choice of colors though. We can discuss elsewhere :)

webviz_subsurface/_components/tornado/_tornado_bar_chart.py
@@ -227,7 +259,8 @@ def layout(self) -> Dict:
"tickfont": {"size": 15},
},
"showlegend": False,
"hovermode": "y",
"hovermode": "closest",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing hovermode from "y" to "closest" causes the "on click" callback in TornadoWidget to fail as it previously received both low and high bars as input, but now only receives the one that is clicked.

This has either to be changed back or atleast set to "y" if the allow_click option is set.
Alternatively investigate if there is some way to separate hovermode from the data received from clickData.
This seems to be a bug/limitation in Plotly.

@tnatt tnatt force-pushed the tornado-improvements branch from b9946ef to a162990 Compare October 25, 2021 08:59
@tnatt tnatt force-pushed the tornado-improvements branch from a162990 to 02aeb0e Compare October 25, 2021 09:10
@HansKallekleiv HansKallekleiv self-requested a review October 25, 2021 09:28
@anders-kiaer anders-kiaer merged commit dc84940 into equinor:master Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HansKallekleiv HansKallekleiv HansKallekleiv approved these changes

@anders-kiaer anders-kiaer Awaiting requested review from anders-kiaer

@asnyv asnyv Awaiting requested review from asnyv

Assignees

@tnatt tnatt

Labels
next release 🚢 To be included in next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tnatt @HansKallekleiv @anders-kiaer