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_t_step not correct for long timespans #803

Open
berland opened this issue Feb 11, 2021 · 5 comments
Open

add_t_step not correct for long timespans #803

berland opened this issue Feb 11, 2021 · 5 comments

Comments

@berland
Copy link
Contributor

berland commented Feb 11, 2021

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
Copy link
Contributor Author

berland commented Feb 11, 2021

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.

@pinkwah
Copy link
Collaborator

pinkwah commented Feb 11, 2021

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

@berland
Copy link
Contributor Author

berland commented Feb 11, 2021

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

@pinkwah
Copy link
Collaborator

pinkwah commented Feb 11, 2021

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.

@berland
Copy link
Contributor Author

berland commented Feb 11, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants