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

MSEED: 1 microsecond rounding issue #1697

Merged
merged 1 commit into from Mar 6, 2017

Conversation

Projects
None yet
3 participants
@krischer
Member

krischer commented Mar 6, 2017

Seems we have a minor rounding issue on master: http://tests.obspy.org/72428/#1

@megies megies added this to the 1.1.0 milestone Mar 6, 2017

@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer Mar 6, 2017

Member

I did not manage to reproduce locally as the test was not reproducible as it used the current timestamp. I guess it was some kind of rounding issue with the new nanoseconds UTCDateTime that by chance manifested in that test run. I replaced the current timestamp with a hardcoded time of the test failure. If this works it should be good enough.

Member

krischer commented Mar 6, 2017

I did not manage to reproduce locally as the test was not reproducible as it used the current timestamp. I guess it was some kind of rounding issue with the new nanoseconds UTCDateTime that by chance manifested in that test run. I replaced the current timestamp with a hardcoded time of the test failure. If this works it should be good enough.

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Mar 6, 2017

Member

I saw a couple of those fails recently, see here: http://tests.obspy.org/?limit=50&show=errors

But yeah, hard to debug with a non fixed test value..

Member

megies commented Mar 6, 2017

I saw a couple of those fails recently, see here: http://tests.obspy.org/?limit=50&show=errors

But yeah, hard to debug with a non fixed test value..

@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer Mar 6, 2017

Member

I tried a couple million timestamps on my machine and I cannot reproduce it. @megies Can you try on debian?

Member

krischer commented Mar 6, 2017

I tried a couple million timestamps on my machine and I cannot reproduce it. @megies Can you try on debian?

@barsch

This comment has been minimized.

Show comment
Hide comment
@barsch

barsch Mar 6, 2017

Member

can't reproduce either - the test does not fail for me

Member

barsch commented Mar 6, 2017

can't reproduce either - the test does not fail for me

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Mar 6, 2017

Member

I also can't reproduce it locally.. let's wait and see what the docker testbot has to say..

Member

megies commented Mar 6, 2017

I also can't reproduce it locally.. let's wait and see what the docker testbot has to say..

@krischer krischer merged commit be0bbcd into master Mar 6, 2017

8 of 9 checks passed

codecov/changes 8 files have unexpected coverage changes not visible in diff.
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 90%)
Details
codecov/project 87.66% (+1.62%) compared to 7fc054e
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
docker-deb-buildbot Deb packaging and testing succeeded
Details
docker-testbot Docker tests succeeded
Details

@krischer krischer deleted the timing-problem branch Mar 6, 2017

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Mar 6, 2017

Member

Hmm.. strange.. still think this might be a rounding issue edge case, maybe in combination with floating point accuracy of the timestamp?

The timestamp you coded in the test is only down to microseconds.. but in the test execution (the one that failed and was reported) it likely had more accuracy that was cut off? Dunno, didn't look into the new UTCDateTime details fully yet..

        # Random date that previously failed.
        dt = UTCDateTime(2017, 3, 6, 4, 12, 16, 260696)
        self.assertEqual(dt, util._convert_mstime_to_datetime(
            util._convert_datetime_to_mstime(dt)))
Member

megies commented Mar 6, 2017

Hmm.. strange.. still think this might be a rounding issue edge case, maybe in combination with floating point accuracy of the timestamp?

The timestamp you coded in the test is only down to microseconds.. but in the test execution (the one that failed and was reported) it likely had more accuracy that was cut off? Dunno, didn't look into the new UTCDateTime details fully yet..

        # Random date that previously failed.
        dt = UTCDateTime(2017, 3, 6, 4, 12, 16, 260696)
        self.assertEqual(dt, util._convert_mstime_to_datetime(
            util._convert_datetime_to_mstime(dt)))
@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer Mar 6, 2017

Member

On my machine I tested it with randomly scattered timestamps in nanoseconds precision. I stopped it after a couple million iterations with no failure.

Member

krischer commented Mar 6, 2017

On my machine I tested it with randomly scattered timestamps in nanoseconds precision. I stopped it after a couple million iterations with no failure.

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Mar 6, 2017

Member

Ok.. let's handle it if we see more of it.. we have lots of tests after all.. :-)

Member

megies commented Mar 6, 2017

Ok.. let's handle it if we see more of it.. we have lots of tests after all.. :-)

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