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

Add seismic intersection module - seismic fence #449

Merged
merged 43 commits into from
Dec 1, 2023

Conversation

jorgenherje
Copy link
Collaborator

@jorgenherje jorgenherje commented Oct 27, 2023

Add first version of intersection module with seismic fence and wellbore path

Based on: Work by @HansKallekleiv in:


TODO:

  • Remove backend/seismic_vds_slice.ipynb
  • Remove unused new code in backend/src/services/sumo_access/_helpers.py and backend/src/services/sumo_access/queries/case.py
  • Cleanup code in frontend/src/modules/Intersection/utils/esvIntersectionControllerUtils.ts
  • Cleanup code in frontend/src/modules/Intersection/utils/esvIntersectionDataConversion.ts

Created new issues:

@jorgenherje jorgenherje self-assigned this Oct 27, 2023
@jorgenherje jorgenherje added the enhancement New feature or request label Oct 27, 2023
@jorgenherje jorgenherje changed the title Add intersection module - seismic fence Add seismic intersection module - seismic fence Nov 2, 2023
@jorgenherje jorgenherje marked this pull request as ready for review November 2, 2023 13:53
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.

Looks good.

Some minor issues that should be fixed:

  • Atleast for vertical wells the image is displaced related to the trajectory.
    image

  • Color palette change does not trigger rerender

  • Wrong fill value for traces outside the cube. See inline comment.

backend/src/services/vds_access/vds_access.py Outdated Show resolved Hide resolved
- Handle fillValue better
- Correct bug for xAxisOffset, as it was extending curtain too much.
- Ensure update of color palette triggers re-render of image
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.

Minor issue. Otherwise good!

frontend/src/modules/SeismicIntersection/settings.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@rubenthoms rubenthoms left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Nice code separation and concise code, good to read and understand. A couple of comments/suggestions.

frontend/src/modules/SeismicIntersection/loadModule.tsx Outdated Show resolved Hide resolved
frontend/src/modules/SeismicIntersection/settings.tsx Outdated Show resolved Hide resolved
frontend/src/modules/SeismicIntersection/settings.tsx Outdated Show resolved Hide resolved
frontend/src/modules/SeismicIntersection/settings.tsx Outdated Show resolved Hide resolved
frontend/src/modules/SubsurfaceMap/view.tsx Outdated Show resolved Hide resolved
frontend/src/modules/SeismicIntersection/view.tsx Outdated Show resolved Hide resolved
frontend/src/modules/SeismicIntersection/view.tsx Outdated Show resolved Hide resolved
frontend/src/modules/SeismicIntersection/view.tsx Outdated Show resolved Hide resolved
Back-end:
- Rename @router.post method to `post_get`
- Add async to function names
- Renamed "seismicDirectory" to "seismicCubeMetaList".
- Update attribute names for clarification
- Update doc for clarification

Front-end:
- Adjust iso date/interval strings conversion
- Adjust "seismic directory" -> "seismic cube meta list" naming.
- Improve naming and typing for readability
Copy link
Collaborator

@rubenthoms rubenthoms left a comment

Choose a reason for hiding this comment

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

LGTM, ready to be merged 👍

@jorgenherje jorgenherje merged commit 69f3a6c into equinor:main Dec 1, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants