#13958 OSDU well path import: recover absolute UTM and target unit system#940
Open
magnesj wants to merge 12 commits into
Open
#13958 OSDU well path import: recover absolute UTM and target unit system#940magnesj wants to merge 12 commits into
magnesj wants to merge 12 commits into
Conversation
Review Summary by QodoOSDU well path import: recover absolute UTM coordinates and support unit conversion
WalkthroughsDescription• Recover absolute UTM coordinates by fetching Well surface origin from OSDU parent record • Detect and convert trajectory length units (feet/meters) to meters before coordinate transformation • Add target unit system selector (Meters/Feet) on import wizard summary page • Implement standalone Python tool to inspect wellbore trajectory data independently Diagramflowchart LR
A["OSDU Wellbore Record"] -->|fetch parent Well| B["Well SpatialLocation"]
B -->|extract surface origin| C["Surface Easting/Northing/CRS"]
D["Trajectory Parquet"] -->|read MD/TVD/X/Y| E["Local Coordinates"]
F["AvailableTrajectoryStationProperties"] -->|detect unit| G["Unit-to-Meters Factor"]
E -->|apply factor| H["Coordinates in Meters"]
C -->|add to X/Y| I["Absolute UTM Coordinates"]
H -->|combine with origin| I
J["Target Unit Selector"] -->|scale result| K["Final Well Path"]
I -->|scale by target unit| K
File Changes1. ApplicationLibCode/Application/Tools/Cloud/RiaOsduConnector.h
|
Code Review by Qodo
1. pd unused import
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+175
to
+183
| import pandas as pd | ||
| import pyarrow.parquet as pq | ||
| except ImportError: | ||
| print("pandas / pyarrow not installed; skipping parquet inspection.", file=sys.stderr) | ||
| return 0 | ||
|
|
||
| table = pq.read_table(io.BytesIO(parquet_bytes)) | ||
| df = table.to_pandas() | ||
| print(f"Columns: {list(df.columns)}") |
There was a problem hiding this comment.
1. pd unused import 📘 Rule violation ⚙ Maintainability
The new script imports pandas as pd but never uses it, which will fail Ruff linting. This violates the requirement that modified/added Python code passes ruff check --fix with no remaining findings.
Agent Prompt
## Issue description
`scripts/tools/extract_osdu_wellpath.py` imports `pandas as pd` but never uses `pd`, which will trigger Ruff's unused-import lint error.
## Issue Context
The PR compliance checklist requires new/modified Python code to be Ruff-linted/formatted (`ruff format` and `ruff check --fix`) with no remaining findings.
## Fix Focus Areas
- scripts/tools/extract_osdu_wellpath.py[175-183]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
725
to
+740
| if ( shouldIncludeTrajectory( w.existenceKind ) ) | ||
| { | ||
| QString wellboreTrajectoryId = w.id; | ||
| auto location = m_osduConnector->requestWellSurfaceLocationBlocking( wellbore.value().wellId ); | ||
| const double targetUnitToMeters = m_targetUnitComboBox->currentData().toDouble(); | ||
| wiz->addWellInfo( { .name = wellbore.value().name, | ||
| .wellId = wellbore.value().wellId, | ||
| .wellboreId = w.wellboreId, | ||
| .wellboreTrajectoryId = wellboreTrajectoryId, | ||
| .existenceKind = w.existenceKind, | ||
| .datumElevation = wellbore.value().datumElevation } ); | ||
| .datumElevation = wellbore.value().datumElevation, | ||
| .surfaceEasting = location.easting, | ||
| .surfaceNorthing = location.northing, | ||
| .crs = location.crs, | ||
| .unitToMeters = w.unitToMeters, | ||
| .targetUnitToMeters = targetUnitToMeters } ); |
There was a problem hiding this comment.
2. Wizard surface fetch deadlock 🐞 Bug ☼ Reliability
WellSummaryPage::updateSummaryDisplay holds m_mutex while calling requestWellSurfaceLocationBlocking(), which runs a nested QEventLoop that processes user input; a re-entrant onFilterChanged()/updateSummaryDisplay will attempt to re-lock m_mutex and freeze the wizard. The surface-location request also has no timeout, so the nested loop can block indefinitely on slow/hung networking.
Agent Prompt
### Issue description
`WellSummaryPage::updateSummaryDisplay()` holds `m_mutex` while calling `requestWellSurfaceLocationBlocking()`, which runs a nested `QEventLoop` that processes user input events and has no timeout. This can deadlock on re-entrancy (e.g., `currentIndexChanged` firing while the loop is running) and can hang indefinitely.
### Issue Context
`RiaCloudConnector::requestTokenBlocking()` already demonstrates a safer approach with a `QTimer` timeout and `ExcludeUserInputEvents`.
### Fix Focus Areas
- ApplicationLibCode/Commands/OsduCommands/RiuWellImportWizard.cpp[704-741]
- ApplicationLibCode/Application/Tools/Cloud/RiaOsduConnector.cpp[803-831]
- ApplicationLibCode/Application/Tools/Cloud/RiaCloudConnector.cpp[346-360]
### Suggested fix
- Do not call blocking network I/O while holding `WellSummaryPage::m_mutex`:
- Copy the needed `m_wellboreTrajectories` data to a local structure under lock, then release the lock before any network calls.
- Add a timeout + exclude user input for the nested loop in `requestWellSurfaceLocationBlocking()` (mirror `requestTokenBlocking()`):
- Use a `QTimer` to quit the loop after a sensible timeout.
- Call `loop.exec(QEventLoop::ExcludeUserInputEvents)` to prevent UI-triggered re-entrancy.
- Consider making surface-location retrieval async (preferred), but the above changes are the minimal safe fix.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
magnesj
force-pushed
the
13958-osdu-wellpath-utm-coordinates
branch
from
063460d to
79dbab3
Compare
…lLocation The wellbore trajectory parquet stores X/Y as offsets relative to the wellbore surface point. To recover absolute UTM coordinates, parse SpatialLocation.AsIngestedCoordinates on both the wellbore and the trajectory record and surface easting/northing/CRS through OsduWellbore and OsduWellboreTrajectory.
Forward the surface easting/northing/CRS extracted from OSDU through RimOsduWellPath, the well import wizard, and the data loader so the parquet reader can shift the trajectory's local X/Y back into absolute UTM coordinates.
Fetches a wellbore record and a wellbore-trajectory record from OSDU, dumps their SpatialLocation, downloads the trajectory parquet and prints the absolute UTM coordinates after applying the wellbore surface origin. Reads server / partition from ~/.resinsight/osdu_config.json and the bearer token from ~/.resinsight/osdu_token.json by default. Useful for validating the import path independently of ResInsight.
Wellbore records do not carry a SpatialLocation block; the surface point lives on the parent Well referenced by Wellbore.WellID. Add a blocking lookup that GETs the Well record from the storage API, parses data.SpatialLocation.AsIngestedCoordinates and caches the result, then populate WellInfo.surfaceEasting/Northing/crs from it instead of from the wellbore parsing path.
When the Wellbore has no SpatialLocation, fall back to fetching the parent Well record so the surface origin is reported alongside the trajectory parquet inspection. Add --full flag to dump the entire data block of each record.
The OSDU well trajectory parquet files specify MD, TVD, X, and Y values in an advertised length unit (e.g., feet or meters). This change detects the unit and converts all trajectory station properties to meters to ensure correct spatial alignment with the well's surface origin and datum elevation, which are consistently handled in meters.
The well summary page now offers a Meters / Feet combobox so the user can pick the unit system the imported well path is stored in. The selection is propagated through WellInfo, RimOsduWellPath, and the parquet reader, which scales the meter-frame coordinates by 1 / target factor before producing the final RigWellPath geometry.
Add shared helpers that parse OSDU UnitOfMeasure ids and the linear unit embedded in a CRS WKT, and apply them at every JSON-parse entry point so the surface origin (from the Well's SpatialLocation) and the wellbore's datum elevation are stored in meters regardless of how OSDU advertises them. Refactor the existing trajectory MD-unit detection to use the same helper. The well-path reader's invariant that surface origin and datum are already meters is now enforced at parse time.
…stem WellSummaryPage now picks Feet as the combobox default when the first loaded RimEclipseCase reports UNITS_FIELD; otherwise it stays on Meters. This matches the imported well path to the case's coordinate frame so users with field-unit cases do not have to flip the selector every time.
The previous wording claimed MD/TVD/X/Y all advertise the same length unit, which is not true for real OSDU records (X/Y are sometimes tagged as "dega" while the values are clearly metric). Document that we treat MD's unit as canonical on purpose and apply it uniformly.
…ation The trajectory's TVD column is referenced to its vertical CRS (typically EPSG:5715 / MSL Depth, positive=down), so Z = -TVD lands the imported well path in the same frame as the simulation well path and the case grid. The previous +datumElevation term shifted every imported point up by the RKB elevation, producing a constant offset (~20 m for typical North Sea wells) between the imported and simulation trajectories. The datum is still preserved on RigWellPath::setDatumElevation as metadata for depth-relative plots, scaled into the target unit system.
magnesj
force-pushed
the
13958-osdu-wellpath-utm-coordinates
branch
from
79dbab3 to
38e57ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wellbore.WellID) and uses itsSpatialLocation.AsIngestedCoordinatesas the surface origin, since the Wellbore record itself does not carry aSpatialLocation. Trajectory parquetX/Y(offsets relative to that origin) are added back to recover absolute UTM coordinates — fixes wells landing at(0, 0).AvailableTrajectoryStationProperties.MD.StationPropertyUnitID(typicallyftorm) and convertsMD/TVD/X/Yto meters before combining with the meter-frame surface origin and datum elevation.Meters/Feet) on the well import wizard's summary page; the resultingRigWellPathis scaled into the chosen unit at read time.scripts/tools/extract_osdu_wellpath.py, a standalone Python tool to fetch wellbore + well + trajectory records and inspect surface origin / parquet contents independently of ResInsight (uses~/.resinsight/osdu_config.jsonandosdu_token.jsonautomatically).Issue: OPM#13958
Test plan
(457121.86, 7322122.99)for the test wellbore on Equinor's swe partition).SpatialLocationstill imports without crashing (will fall back to origin0, 0with a warning in the log).python scripts/tools/extract_osdu_wellpath.py --wellbore-id ... --trajectory-id ...and confirm the printed absolute UTM matches what ResInsight imports.