-
Notifications
You must be signed in to change notification settings - Fork 59
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
Added average rates and cumulatives over specific time interval to ReservoirSimulationTimeSeries #373
Conversation
5ab52a7
to
764ea7c
Compare
Will delay this one until #386 is merged. Due to the download feature introduced in #386, I feel like the natural solution is to switch the time step to the same format as regular Eclipse vectors (backfilled) for average rates and interval cumulatives, even though fmu-ensemble do the opposite for average rates (get_volumetric_rates). |
c8a8043
to
7a343a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. Some early feedback:
E.g. the values 01.01.2000 for monthly frequency is in fact the value for January 2000. This is opposite to the convention for Eclipse rates, but the same as what is used for get_volumetric_rates in fmu-ensemble. Visualization wise this is not a big issue, as we can control which way we fill, but for a data dump to e.g. csv, this could be confusing for users (e.g. if you dump together with other vectors, and values on the same line/date don't add up). But also confusing if results are not comparable with fmu-ensemble which many users are already used too.
Would "upscaling" also the related time data be feasible technically + make it less dagengerous for misunderstanding? E.g. 2020.01 instead of 2020.01.01 is January 2020 interval/average.
I was a bit confused in the GUI when turning on "Average rates" / "Monthly cumulative" (had selected a cumulative + rate vector, but nothing changed, since I hadn't selected one with prefix AVG_
/ INTL_
). Deactivate those controls (grey out) when an AVG_
/ INTL_
vector is not selected? Another thing to consider (not sure if it is a good idea or bad idea) could be that the dropdown is not extended with AVG_
and INTL_
vectors, but when the user activates calculations from cumulative vectors, any selected cumulative vectors are shown instead with their associated derived vector in the plot?
Question 1:
First: As commented later, this was reverted to make the behavior similar to Eclipse output. E.g. 2020.01.01 with monthly output would in fact be data for December 2019, similar to how a rate like FOPR in Eclipse would be stored. Did this because different behavior between original and derived vectors was too confusing, especially when downloading data too csv or looking at tooltips (the plot itself is easilly handled by line styling of course). To answer your question: As we are handling datetime objects, they have to be stored with precise dates. To reduce the output to e.g. 2020.01 would mean that we have to make a robust method of converting deltas between datetimes to the most descriptive string to use in the csv-export + as tooltips, axis labels and etc.I don't think the behavior here is more or less confusing than the already existing Eclipse output format, but that is of course a bit confusing in itself ;) Question 2:
The checkboxes actually add and remove the AVG_ and INTVL_ options from the list of vector options. Greying them out would therefore not make sense if I understand your question correctly. Can agree that the behavior is not very intuitive, and migth be enough to just have the radio buttons to choose intervals, and leave the AVG_ and INTVL_ vectors always available as selectable vectors? But I think it is useful to have the prefixes. Both to be able to plot e.g. FOPR and AVG_FOPR together, and to reduce the risk of confusion/errors (especially when data is downloaded to csv and the vector descriptions are gone). |
2b5079a
to
4a895aa
Compare
b4d57f2
to
ab96238
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Some final small comments - otherwise I think this is ready for merge 🚀
…val to ReservoirSimulationTimeseries. The calculation is made from cumulative vectors on Eclipse format.
3a47883
to
4de2faf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀🎸
…inor#373) Co-authored-by: Havard Bjerke <[email protected]>
Closes #292
The PR covers statistically averaged rates (per day) for a time period calculated from cumulative (total) vectors (In the petroleum industry commonly known as
calendar day rates
).In addition the delta production per time interval can be shown, which of course is the equivalent of a rate with the interval length as time unit.
To make it performant on-the-fly, only resampling is supported, not interpolation/extrapolation, meaning that the intervals have to be the same or a coarser interval than the dataset, and that the dataset has to be on a recurring frequency. Some modifications to verification of the frequency option part remains, but the plan is to support input frequencies:
daily
: with available intervalsdaily
,monthly
andyearly
(note thatdaily
quickly grows to large amounts of data, so might be slow).monthly
(first day of month, pandas MS): with available intervalsmonthly
andyearly
yearly
(first day of year, pandas YS): with available intervalsyearly
Currently the delta per interval and average rates are calculated back to the first day of the interval. E.g. the values 01.01.2000 for monthly frequency is in fact the value for January 2000. This is opposite to the convention for Eclipse rates, but the same as what is used for
get_volumetric_rates
infmu-ensemble
. Visualization wise this is not a big issue, as we can control which way we fill, but for a data dump to e.g. csv, this could be confusing for users (e.g. if you dump together with other vectors, and values on the same line/date don't add up). But also confusing if results are not comparable with fmu-ensemble which many users are already used too 🤯...When it comes to the inclusion in plugins, this PR shows a prototype for the ReservoirSimulationTimeSeries plugin. Attempted a few different solutions, but think including the new options in the regular vector selectors was the cleanest way I could find.
Has been proposed to force delta production over intervals (those starting with INTVL_) to be bar charts, but that turned out to not work well when plotting as realizations, and it was also not ideal to handle statistics due to overlaying bars.