Fix date for long simulation times

[totto82]

No description provided.

[atgeirr]

Actually, since simulationTimeElapsed() returns a double, you don't need the cast. Makes me wonder why it was used in the first place!

[andlaus]

probably to avoid some stupid compiler warning. if so, casting tolong long int should help.

[atgeirr]

If seconds() expects an integer type, we are limiting ourselves to 1-second timestep resolution, which is not ideal, perhaps we should use microseconds instead (if it's a 64 bit integer at least).

[totto82]

When I removed the (double) the dates are messed up again.

[totto82]

Sorry I don't think it was the removing of the (double) that messed it up.
If I do:

boost::posix_time::minutes( simulationTimeElapsed() / 60);

it seems to work. Maybe it is time to make a test for this call?

[andlaus]

hm, looks like boost::posix_time::{hours,minutes,seconds,...} expect long as their argument. that's a bit unfortunate API design IMO, but we could lessen the impact of it by replacing above line with

 return startDateTime() + boost::posix_time::hours(static_cast<long>(simulationTimeElapsed()/3600)) + boost::posix_time::hours(static_cast<long>(std::fmod(simulationTimeElapsed(), 3600));

that's still not ideal because it extends the maximum time by "only" factor 3600. though.

[atgeirr]

A long is 64 bits on all platforms we support, so it should have plenty resolution even at milliseconds (famous last words). We do not want to use (only) the minutes() helper, as that limits the shortest timestep to one minute (since it takes integer arguments). I think seconds() as in the current version of the PR is fine, but we could hit a problem with wanting to use shorter timesteps at some point.

@andlaus your suggestion is buggy, I assume you meant seconds() for the second call. In any case, I don't think it matters at all with 64 bits, do you?

[andlaus]

A long is 64 bits on all platforms we support.

Right. I confused that with the fact that the normal integers are still 32 bits on x86 and that only long long is guaranteed to be 64 bits.

but in this case: 2^63 seconds is almost 300 billion years, so why does the current code go belly-up? (I assume @totto82 did not want to simulate until the end of the universe. ;))

@andlaus your suggestion is buggy, I assume you meant seconds() for the second call. In any case, I don't think it matters at all with 64 bits, do you?

yes, that was a copy-and-pasto.

[joakim-hove]

2^63 seconds is almost 300 billion years, so why does the current code go belly-up?

I think boost internally has a 32 bit problem, so in the parser we stoped using boost datetime for long simulations: OPM/opm-parser#1092, OPM/opm-parser#1102 and OPM/opm-parser#1104 - seems the simulator pulls the issue back in?

[andlaus]

ok, to me this is a perfect reason to rip out boost::posix_time and replace it with std::chrono. maybe I missed something, though.

[atgeirr]

Just to be clear @totto82, the current version (that just removes the explicit int cast) solves your problem?

[totto82]

Just to be clear @totto82, the current version (that just removes the explicit int cast) solves your problem?

No it doesn't.

[atgeirr]

No it doesn't.

Then I am truly confused. So seconds( (double) simulationTimeElapsed() ) works, but seconds( simulationTimeElapsed() ) does not, in spite of that method returning a double! Someone hit me in the head with a clue hammer please...

[akva2]

that implies that seconds() is overloaded and that it does something different for ints no?

[totto82]

Then I am truly confused. So seconds( (double) simulationTimeElapsed() ) works, but seconds( simulationTimeElapsed() ) does not, in spite of that method returning a double! Someone hit me in the head with a clue hammer please...

No both does't work. I guess this is an issue in boost as Joakim points out.

[totto82]

As @joakim-hove points out there is an issue with boost::posix_time::seconds using 32 bit long, fortunately it works if you use ::milliseconds.

Now it works until 31.12.9999.

[atgeirr]

Now it works until 31.12.9999

I hope that will be sufficient!

[atgeirr]

jenkins build this please