resdata icon indicating copy to clipboard operation
resdata copied to clipboard

add_t_step not correct for long timespans

Open berland opened this issue 4 years ago • 5 comments

Stepping 500 year forward in time, the EclSumTStep is off by ~5 minutes:

import datetime

from ecl.summary import EclSum

from ecl.summary import EclSum

# Initialize an EclSum object with startime at 2000-01-01:
ecl_sum = EclSum.writer("FOO", datetime.datetime(2000, 1, 1), 1, 1, 1)

# Step forward to 2500-01-01:
days = (datetime.date(2500, 1, 1) - datetime.date(2000, 1, 1)).days
assert days == 182622  # 365.244 days pr year avg.

t_step = ecl_sum.addTStep(1, days)
print(t_step)

# Gives:
# EclSumTStep(sim_days=182621.99703703704, sim_time=2499-12-31 23:55:44, report=1, ministep=4) at 0x33f71f0

berland avatar Feb 11 '21 07:02 berland

~This problem occurs from 2068 and onwards. Y2K problem?~

This problem starts after stepping forward ca 68 years, starting from either f.ex year 2000 or year 2100.

berland avatar Feb 11 '21 08:02 berland

EclSum's add_t_step doesn't deal with eg. leap seconds. This would necessitate a refactor which isn't feasible with our budget of 0kr.

https://github.com/equinor/ecl/blob/007f000ba97b04b670217aa425220f907c022e79/python/ecl/summary/ecl_sum.py#L307-L308

pinkwah avatar Feb 11 '21 10:02 pinkwah

Leap seconds was my hypothesis too, but I have failed to understand where they enter the calculation and thus how it causes this.

berland avatar Feb 11 '21 10:02 berland

Okay nevermind. It's due to floating-point conversions. ecl accepts simulation-seconds as a 32-bit float. The number of seconds don't fit into this and thus it gets automatically rounded down:

import ctypes
import datetime

x = datetime.datetime(2500, 1, 1)
y = datetime.datetime(2000, 1, 1)

# from EclSum.add_t_step
sec = (x - y).days * 24 * 60 * 60    # => 15778540800
sec_f32 = ctypes.c_float(sec).value  # => 15778540544.0

minute_diff = (sec - sec_f32) / 60                       # => ~ 4.267
computed_date = y + datetime.timedelta(seconds=sec_f32)  # => datetime.date(2499, 12, 31)

This might still not be fixable in short order due to our requirement for C ABI compatibility, and changing the type from float to double would break it.

pinkwah avatar Feb 11 '21 10:02 pinkwah

Thanks, makes sense. No apparent workaround. If this only affects synthetic UNSMRY it is not of high priority.

berland avatar Feb 11 '21 11:02 berland