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.

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

Well Log Viewer: Expose computed track info values #2196

Closed
Anders2303 opened this issue Aug 22, 2024 · 18 comments
Closed

Well Log Viewer: Expose computed track info values #2196

Anders2303 opened this issue Aug 22, 2024 · 18 comments
Assignees

Comments

@Anders2303
Copy link
Collaborator

For the Webviz project, we're using the Well Log Viewer component, but we want to create our own track info panel (as the included one doesnt fit our layout approach). W are

Currently, we're using the main WellLogViewer (which is believe is wrapping the WellLogViewerWithScroller) component. Unforunately the available onInfo callback does not fire, as the component instead redirects that event to the internal callbackManager. However, even if I DID get the onInfo callback, it only provides the generic x-cordinate (as opposed to the actual MD value), and I still need to manually compute each track value, and so on.

All of this is already being calculated inside the component logic, could you introduce a way to access the computed track info? Either via a callback, or a getter function in WellLogController.?

@hkfb
Copy link
Collaborator

hkfb commented Aug 22, 2024

I remember vaguely some discussions about a callback for readout values, do you remember @mirisb?

@mirisb
Copy link
Collaborator

mirisb commented Aug 23, 2024

@hkfb yes, there is a callback in WLV controller that can be used to get selection (MD) and use it to generate readout (or do whatever you want with it). onContentSelection works very good for us: since we have all information about the logs (that we compile and send to the internal component) we can use the selected MD to find the log values in all tracks and wells. We generate ourselves nearly identical values to the generated internal info values and can choose whether to display them or do something else with it

@Anders2303
Copy link
Collaborator Author

Yeah, I was planning on using onContentSelection myself if no other option was available. It's just a little bit unfortunate that I will need to essentially rewrite the "MD to curve data" lookup logic myself, since that already exists within the component code.

@hkfb
Copy link
Collaborator

hkfb commented Aug 23, 2024

I think it would make sense to return the curve data itself like @Anders2303 suggests. It's more consistent with eg. the subsurface-viewer component, and it would ensure consistency when reading an interpolated well trajectory or curve.

@Anders2303 would you be interested in creating a patch?

@Anders2303
Copy link
Collaborator Author

@hkfb Sure! Can probably find time for that sometime next week

@Anders2303
Copy link
Collaborator Author

Anders2303 commented Aug 26, 2024

Just to make sure we're on the same page, @hkfb, should I make it so the existing onInfo event returns the fully calculated curve data, or should I go with a new event all together, eg onComputedData or similar?

@Anders2303
Copy link
Collaborator Author

... well actually, thinking about it; I probably shouldn't change the emitted properties on onInfo, since that would be a breaking change. So I'll go for the new event instead.

@hkfb
Copy link
Collaborator

hkfb commented Aug 26, 2024

... well actually, thinking about it; I probably shouldn't change the emitted properties on onInfo, since that would be a breaking change. So I'll go for the new event instead.

I wouldn't necessarily consider extending onInfo a breaking change. Can you show an example what it would look like?

@Anders2303
Copy link
Collaborator Author

No that's a fair point, it could just be a simple extension. I was just overthinking it...

I was picturing something like this

    setInfo(x: number = Number.NaN): void {
        // ...
        // ...
        const iFrom = this.getTrackScrollPos();
        const iTo = iFrom + this._maxVisibleTrackNum();

        const curveInfo = /* Get data from fillInfo utility */
        this.props.onInfo(curveInfo);
    }

but as you said, it only needs to be a simple extension, so we can just do something like this:

    setInfo(x: number = Number.NaN): void {
        // ...
        // ...
        const iFrom = this.getTrackScrollPos();
        const iTo = iFrom + this._maxVisibleTrackNum();

        const curveInfo = /* Get data from fillInfo utility */
        this.props.onInfo(x, this.logController, iFrom, iTo, curveInfo);
    }

Sidenote; The actual computation of the curve data currently happens inside the WellLogInfoPanel component (as part of its own onInfo listener). This does mean that the computation only happens if the info panel is mounted. Moving it to the setInfo block means the computation will run all the time, even if the curve data is not needed. Is this okay?

@hkfb
Copy link
Collaborator

hkfb commented Aug 26, 2024

Sidenote; The actual computation of the curve data currently happens inside the WellLogInfoPanel component (as part of its own onInfo listener). This does mean that the computation only happens if the info panel is mounted. Moving it to the setInfo block means the computation will run all the time, even if the curve data is not needed. Is this okay?

It could be a heavy calculation given many tracks and curves, so it would be preferable to avoid the computation if it's not needed. Perhaps your initial suggestion to add a new callback is better?

@Vladimir-Kokin any thoughts?

@Anders2303
Copy link
Collaborator Author

Check out 395c128

I implemented the new event approach here, to experiment a little. I added a new event to the callback manager, called "onInfoFilled", and then moved the old fill-info logic from the WellLogInfoPanel component up to the WellLogViewer component. To avoid unecessary computations, WellLogViewer will only perform the fill-info logic if the callback manager has any registered onInfoFilled callbacks. The old onInfo is also stilled called as before.

I think this is a clean solution, and should make it easier for people to make their own info-panels going forward. What do ya'll think?

If you give the go-ahead, apply the same changes to SyncLogViewer, and throw up a PR

@hkfb
Copy link
Collaborator

hkfb commented Aug 26, 2024

LGTM. Please also make a simple Storybook example that demonstrates the usage.

@Anders2303
Copy link
Collaborator Author

Not really sure to what extent I can make a valuable storybook example? I've just reorganized a little to expose a new event... Should it be a storybook example showing a custom made information panel?

@hkfb
Copy link
Collaborator

hkfb commented Aug 26, 2024

Should it be a storybook example showing a custom made information panel?

Yes, a simple custom panel that demonstrates the readout.

@Anders2303
Copy link
Collaborator Author

PR for patch is up! #2205

@hkfb
Copy link
Collaborator

hkfb commented Aug 27, 2024

Thanks, will take a look.

hkfb added a commit that referenced this issue Aug 28, 2024
Solves #2196 

1. Moved the "fillInfo" computation from the `WellLogInfoPanel`
component up to the WellLogViewer and SyncLogViewer core components
2. Added a `onInfoFilled` event to the `CallbackManager`, and made the
info-panel listen to that instead
3. Made the viewer-components emit a `onInfoFilled` event, to expose the
interpolated data to external components
1. Story-book example at
`/story/welllogviewer-demo-welllogviewer--on-info-filled-event`

---------

Co-authored-by: Håvard Bjerke <havard.bjerke@aspentech.com>
hkfb pushed a commit that referenced this issue Aug 28, 2024
@mirisb
Copy link
Collaborator

mirisb commented Sep 1, 2024

@Anders2303 , @hkfb
Question - is the new even only returning info for a single well? what happens for syncLogViewer that is showing multiple wells? will the event be called multiple times for each iWellLog?

@Anders2303
Copy link
Collaborator Author

Anders2303 commented Sep 2, 2024

@mirisb
It will fire once per iWellLog. This should match how other events are called for that component.

hkfb pushed a commit that referenced this issue Sep 3, 2024
… mounted hook (#2217)

Resolves issue #2216 

The callback manager events introduced in PR #2196 where being set up in
the _constructor_ hook, not in the `componentDidMount()` hook, which is
against React guidelines.

This PR fixes that
hkfb pushed a commit that referenced this issue Sep 3, 2024
## [1.12.7](https://github.com/equinor/webviz-subsurface-components/compare/well-log-viewer@1.12.6...well-log-viewer@1.12.7) (2024-09-03)

### Bug Fixes

* Well Log Viewer - Moved callback subscription to well-log-viewer mounted hook ([#2217](#2217)) ([c0e5c97](c0e5c97)), closes [#2216](#2216) [#2196](#2196)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants