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

New simulation time series plugin #867

Conversation

jorgenherje
Copy link
Collaborator

@jorgenherje jorgenherje commented Dec 1, 2021

Create new simulation time series plugin SimulationTimeSeries

Based on the functionality in original ReservoirSimulationTimeSeries plugin, with additional functionality requests listed in following issue: #698.

This PR includes and closes parts of the listed requests:

  • Show multiple delta ensembles - create delta ensembles
  • Stacking possibility - subplot per vector and subplot per ensemble

Review feedback

Please give feedback on following points:

  • Naming of objects in _property_serialization-folder
  • Naming of objects in types-folder
  • Can utility functions in utils-folder be handy for others and placed in webviz-subsurface/_utils-folder?
  • Consider moving ProviderSet to webviz_subsurface/_utils-folder. Could be reusable, and additional functionality can be added based on needs.

Summary

The work includes structuring and refactoring of code for enhanced plugin implementation. Goal is to have code separated on functionality (callbacks, layout, business logic etc.), convert dash property to strongly typed arguments for "business logic". Clarifies intended usage and testability, preventing loosely typed arguments resulting in passing incorrect data.

Utilize best practice example in webviz-plugin-boilerplate.


Main points

  • Use ensemble summary data providers - Ensemble summary provider #721
    • Allows resampling frequency selectable
  • Refactor and structure code for readability
    • Separate code dependent - layout, callback and business logic used in callback
    • Improve testability unit tests
    • Use boilerplate best practice plugin - webviz-plugin-boilerplate
    • Introduce strongly typed functionality for enhanced readability and more strict interfaces
  • Show multiple Delta Ensemble:
    • Create Delta Ensemble
    • Delta Ensemble as selectable ensemble
  • Group by:
    • vectors - subplots per selected vector
    • ensemble - subplots per selected ensemble

Todo

  • Unit testing of "business logic"

sigurdp added 30 commits May 25, 2021 16:13
Copy link
Collaborator

@anders-kiaer anders-kiaer left a comment

Choose a reason for hiding this comment

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

Great work @jorgenherje 👏🚀 Looks nice!

I like that you also apparently has laid the ground for reusing parts of the functions in other plugins as well going forward.

There are some TODOs in the code (I've only commented on a couple of them). Should we stick with them in code only, or should we make issue of them as well as reminders?

Besides that I have only a couple of minor comments, then this is ready to merge IMO when we see green light towards example data.

jorgenherje and others added 8 commits December 15, 2021 12:51
- Removing incorrect comments (Mostly TODO's)
- Have made GitHub-issues based on some of the comments
Rename from DerivedEnsembleVectorsAccessor to
DerivedVectorsAccessor
… on invalid frequency.

- Due to user request, the verification is removed, and user
must be aware tha vector metadata incosistency can occur.
Note retrieveing of metadata gives first valid metadata, thus
invalid metadata should not affect the plugin.

- Add ValueError when converting input sampling str to Frequency results
in `None` i.e. raw data.

- Fix incorrect comment
@jorgenherje jorgenherje merged commit ab1a833 into equinor:master Dec 16, 2021
VincentNevermore pushed a commit to VincentNevermore/webviz-subsurface that referenced this pull request Jul 19, 2022
…or#957)

* 1. Removed unnecessary viewState management, using initialViewState to let deckGL manage the state
2. Set axes rotation along Y axis for 3D view
Fix equinor#956
Fix equinor#867

* update snapshot

* capture screenshots

* capture screenshots

* new screenshots updated

Co-authored-by: Shadab Khan <[email protected]>
Co-authored-by: Sanghavi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CeeSol Task owned by Ceetron Solutions enhancement 🚀 New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants