[ri-cloud-api]: Add grid data from sumo using api - #14653
Conversation
25b52a8 to
a62f3cd
Compare
|
When an ensemble is created, auto-select the first grid if there are grids available. This will ensure that the creation of a grid ensemble is a one-click operation. |
Code review finding: prefetch-batch failures can be silently re-requestedFile: Only the displayed step ( If one of those non-displayed prefetch steps fails, Impact: Not a crash, but it partially undermines the stated "stop retrying failures" fix for prefetch-batch members: repeated failed network requests can occur as the user scrubs near (not necessarily onto) a time step whose data failed to load. Suggested fix: Write a placeholder (or track a distinct "failed" set) for every step in the prefetch batch, not just the displayed one, so |
magnesj
left a comment
There was a problem hiding this comment.
Some minor changes, otherwise good to go
|
Rif* files should probably be located inside FileInterface, not Application? |
|
Follow-up on comment from @jonjenssen The Rif* file should be moved to the FileInterface folder. GUI logic should be moved out of Rif* classes. Here is one option using callbacks: |
Grid geometry and grid properties can now be read from Sumo through the local ri-cloud-api service, alongside the summary data that was already there. A Sumo data source can be turned into a grid ensemble, whose cases load their geometry and their property time steps on demand. The requests live in RiaSumoGrid, a data delegate of the connector alongside the ones for discovery and summary data. Downloaded property blobs are held in RiaSumoBlobCache, bounded by total size so a long session on a large case cannot grow without limit, and the time steps of a property are fetched in concurrent batches rather than one round trip at a time. Computing the mobile volume weighted mean is made optional, and is off for Sumo grid ensembles. It reads PORV and the saturation of every phase, so opening a case with SOIL displayed pulled down several properties nobody had asked for. Do not read the authentication objects from the transfer thread when loading grid data The blob id lookup for a grid property runs on the transfer thread and built its Authorization header by calling token(), which reads the QOAuth2AuthorizationCodeFlow owned by another thread. It now uses the copy taken before the work was handed over, as the summary and blob requests already do. Add usage of new endpoint and RimReservoirGridEnsembleSumo Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Show Sumo property loading in the 3D view info box - Blank cells while a grid property is transferred look the same as cells with no data. Name what is on its way in the view overlay instead, so the reason is visible where the user is already looking. - Also report Sumo property loading in the status bar - Put the loading banner up when the view first reads a pending time step - Stop aborting queued blob id requests, and stop retrying failures - downloadBlobAsync put a flat 10 second deadline on the blob id request regardless of what the caller asked for. The network manager serves only a few requests per host at a time and the deadline runs from when a request is created, not from when it is served, so a batch of transfers had most of its blob id requests aborted while still queued. The blocking path puts no deadline on these at all, see downloadBlobs, which is why it did not show the problem. The blob id request now follows the same policy as the transfer it belongs to: the generous timeout is a leak guard, not a deadline. - Clearing the slot on failure made this worse. A cleared slot is read again on the next redraw, and redraws are frequent, so one failure became a flood of requests, which caused more timeouts. The placeholder is left in place instead, matching the intended behaviour that a failed result is not retried on every redraw. A time step that genuinely fails now stays blank until the case is reloaded.
- Add timeout for lookup, grid property transfer - Max in flight property transfers (queue additional)
…ime steps to fetch Add placeholder for every slot to keep correct track of state.
…cking races - Fix cancellation of ongoing requests/downloads when new case/realization is selected. - Fix failing grid request for contour map if a default primary case is requested while view already requests/downloads grid properties/data in view. Four further fixes to the per-owner cancelGroup/trackReply registry in RiaSumoConnector and the cases/readers that use it, addressing review comments on the Sumo async grid/property fetch work: 1. RimEclipseView::fieldChangedByUi only checked previousSumoCase->reservoirViews().empty() before closing the case switched away from, missing that RimEclipseCase::reservoirViews() excludes contour map views (those live in contourMapViews()). Switching case in any view could wipe a RimRoffCaseSumo still shown in a different contour map view, discarding its loaded data. Now checks contourMapViews().empty() as well, and contourMapViews() is made public alongside reservoirViews() for this to be usable from RimEclipseView. 2. Switching away from a case used to call closeReservoirCase(), which also discards already loaded grid and result data just to cancel in-flight transfers - forcing a full reload if the user switches back to a realization that had already finished loading. Extracted the cancel-only part into a new RimRoffCaseSumo::cancelPendingTransfers(), used by fieldChangedByUi instead of closeReservoirCase(), which still calls cancelPendingTransfers() internally before discarding data. 3. RimRoffCaseSumo::~RimRoffCaseSumo() was empty, unlike RifReaderSumoGridProperty and RimSummaryEnsembleSumo which both cancel their transfer group in their destructors. Deleting a case without closing it first (e.g. removed from the project tree) left its in-flight startPropertyFetch transfer running in the background. 4. A reply was created and tracked as two separate steps (manager->get() then trackReply()), leaving a gap in which a cancelGroup call landing on another thread could miss the new reply. Replaced with RiaSumoConnector::getAndTrackReply(), which creates and registers the reply under the same mutex cancelGroup locks, so there is no such gap. Threaded an optional cancelGroup parameter through the helpers that create requests (RiaSumoGrid::makePropertyBlobIdRequest, RiaSumoSummary::makeVectorBlobIdRequest/makeParameterBlobIdRequest).
…on Sumo cases RimStatisticsContourMap::computeStatisticsForMaps opens each realization case with no view attached, so nothing drives the normal eager result-loading path used when a property is displayed in a 3D view - reading a result for statistics found no data and returned undefined values instead of triggering a fetch. First attempt: warm up the requested dynamic result via loadResult() before reading it, then poll/wait for the async Sumo fetch to complete (RimRoffCaseSumo::dataLoadingText(), a waitForPendingCaseData() loop in RimStatisticsContourMap using a QEventLoop/QTimer). This worked but downloaded and waited for every time step of the property, most of which the statistics computation never needed for the case being processed. Replaced with a narrower prefetch: added RifReaderSumoGridProperty::prefetchDynamicResult(), which fetches and decodes exactly one requested time step synchronously and writes it directly into the case's result storage, and a RimEclipseCase::prefetchDynamicResult() virtual (no-op by default) overridden in RimRoffCaseSumo to call it. RimStatisticsContourMap now sets setEagerResultLoadingEnabled(false) on the statistics result definition and calls prefetchDynamicResult() for exactly the local time step each active contour map needs before extractCaseResults(), instead of warming up and polling for the whole property.
…or reentrancy crash Three fixes made while testing and fixing issues found after the file-move/GUI-decoupling commits above: 1. Remove the 8-step prefetch batch cap. RifReaderSumoGridProperty::requestTimeStepsAsync/timeStepsToFetch capped the look-ahead batch to RiaSumoDefines::gridPropertyPrefetchBatchSize() (8) with a low-water-mark refill heuristic (gridPropertyPrefetchLowWaterMark(), 4). Removed: timeStepsToFetch() now returns every missing time step of the property, and requestTimeStepsAsync() requests all of them in one batch, relying on QNetworkAccessManager's own per-host connection limit to bound actual concurrent transfers. gridPropertyPrefetchBatchSize()/gridPropertyPrefetchLowWaterMark() removed from RiaSumoDefines as unused. 2. Remove the waitForPendingCaseData() poll loop from RimStatisticsContourMap, now that fetching blocks by design instead. RifReaderSumoGridProperty::prefetchDynamicResult() (and the RimEclipseCase/RimRoffCaseSumo virtual above it) now take a const std::vector<size_t>& stepIndices instead of a single size_t: every not-yet-loaded step is requested together in one requestTimeStepsAsync() batch so the transfers run in parallel, then waitForTimeStepToArrive() is called once per requested step (a no-op for any step already resolved by the time its turn comes) so the call blocks until every requested step has arrived or timed out before returning. RimStatisticsContourMap::computeStatisticsForMaps() now collects every local time step each active context's property needs into a std::map<QString, std::vector<size_t>> keyed by result name, then calls prefetchDynamicResult() once per result name with the full, de-duplicated, sorted list of steps that property needs for this realization - one blocking call per property per case instead of one blocking call per time step, with no separate polling wait needed afterward. The warmUpDynamicResult()/waitForPendingCaseData() free functions and their now-unused includes were removed. 3. Fix destructor reentrancy crash canceling a reader's own in-flight transfers. Crash reported computing contour map statistics, inside RifReaderSumoGridProperty::~RifReaderSumoGridProperty()'s cancelGroup() call, reached via RigEclipseCaseData::~RigEclipseCaseData() releasing its reader while a batch it had requested was still in flight. Every completion lambda in this reader captures a std::weak_ptr<bool> to m_lifetimeToken and gates on it being expired to detect "the reader is gone", but m_lifetimeToken is a member of the reader itself so it does not expire until after the destructor body finishes - cancelGroup()'s QNetworkReply::abort() can trigger a synchronous finished() that re-enters the reader's own completion lambda while the destructor is still on the stack, dereferencing the case data one frame further up the same call stack. Fixed by capturing m_lifetimeToken.get()'s raw pointer identity, then calling m_lifetimeToken.reset() before cancelGroup(), so every isAlive.expired() check - even one reached through a reentrant finished() during that very call - correctly observes "gone". Applied the equivalent reordering to RimRoffCaseSumo::cancelPendingTransfers() and RimSummaryEnsembleSumo's destructor as defense in depth.
7323434 to
2ec618d
Compare
magnesj
left a comment
There was a problem hiding this comment.
Very good, this works as expected on my system.
magnesj
left a comment
There was a problem hiding this comment.
Fix the build errors due to missing includes on Linux
Loads Eclipse grid data (roff) from Sumo as a RimReservoirGridEnsemble, one case per realization.