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.

Sign up for GitHub

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

Jump to bottom

Time Pattern Boundaries not handled correctly #78

Open
DillonJ opened this issue Oct 13, 2021 · 8 comments
Open

Time Pattern Boundaries not handled correctly #78

DillonJ opened this issue Oct 13, 2021 · 8 comments
Labels
bug Something isn't working temporals

Comments

@DillonJ
Copy link
Contributor

DillonJ commented Oct 13, 2021 •
edited
Loading

When using time patterns, the boundaries do not appear to be handled correctly.

E.g. if I write

H1-2 0
H3-5 100
H6-24 0.

I expect the value of 100 to apply for hours 3, 4 and 5 and then zero otherwise. Instead what is happening is that I get 50 for hour 3, 100 for hour 4 and 5, and 50 for hour 6.

I believe what is happening is that somehow, in the above example, it thinks the hour 3 has the value 0 and the value 100 (perhaps).

The attached project zip file illustrates the problem

TimePatterns.zip

Using the following time pattern :

image

Results in the following

2000-01-01T00:00:00	0
2000-01-01T01:00:00	0
2000-01-01T02:00:00	0
2000-01-01T03:00:00	0
2000-01-01T04:00:00	0
2000-01-01T05:00:00	0
2000-01-01T06:00:00	0
2000-01-01T07:00:00	140
2000-01-01T08:00:00	280
2000-01-01T09:00:00	280
2000-01-01T10:00:00	280
2000-01-01T11:00:00	280
2000-01-01T12:00:00	280
2000-01-01T13:00:00	280
2000-01-01T14:00:00	280
2000-01-01T15:00:00	280
2000-01-01T16:00:00	280
2000-01-01T17:00:00	140
2000-01-01T18:00:00	0
2000-01-01T19:00:00	0
2000-01-01T20:00:00	0
2000-01-01T21:00:00	0
2000-01-01T22:00:00	0
2000-01-01T23:00:00	0

I am expecting:

2000-01-01T00:00:00	0
2000-01-01T01:00:00	0
2000-01-01T02:00:00	0
2000-01-01T03:00:00	0
2000-01-01T04:00:00	0
2000-01-01T05:00:00	0
2000-01-01T06:00:00	0
2000-01-01T07:00:00	280
2000-01-01T08:00:00	280
2000-01-01T09:00:00	280
2000-01-01T10:00:00	280
2000-01-01T11:00:00	280
2000-01-01T12:00:00	280
2000-01-01T13:00:00	280
2000-01-01T14:00:00	280
2000-01-01T15:00:00	280
2000-01-01T16:00:00	280
2000-01-01T17:00:00	0
2000-01-01T18:00:00	0
2000-01-01T19:00:00	0
2000-01-01T20:00:00	0
2000-01-01T21:00:00	0
2000-01-01T22:00:00	0
2000-01-01T23:00:00	0

Using this time pattern

image

Results in this

2000-01-01T00:00:00	280
2000-01-01T01:00:00	280
2000-01-01T02:00:00	280
2000-01-01T03:00:00	140
2000-01-01T04:00:00	0
2000-01-01T05:00:00	0
2000-01-01T06:00:00	0
2000-01-01T07:00:00	0
2000-01-01T08:00:00	0
2000-01-01T09:00:00	0
2000-01-01T10:00:00	140
2000-01-01T11:00:00	280
2000-01-01T12:00:00	280
2000-01-01T13:00:00	280
2000-01-01T14:00:00	140
2000-01-01T15:00:00	140
2000-01-01T16:00:00	0
2000-01-01T17:00:00	0
2000-01-01T18:00:00	0
2000-01-01T19:00:00	0
2000-01-01T20:00:00	0
2000-01-01T21:00:00	0
2000-01-01T22:00:00	140
2000-01-01T23:00:00	280
```

I am expecting this:

```
2000-01-01T00:00:00	280
2000-01-01T01:00:00	280
2000-01-01T02:00:00	280
2000-01-01T03:00:00	0
2000-01-01T04:00:00	0
2000-01-01T05:00:00	0
2000-01-01T06:00:00	0
2000-01-01T07:00:00	0
2000-01-01T08:00:00	0
2000-01-01T09:00:00	0
2000-01-01T10:00:00	280
2000-01-01T11:00:00	280
2000-01-01T12:00:00	280
2000-01-01T13:00:00	280
2000-01-01T14:00:00	280
2000-01-01T15:00:00	0
2000-01-01T16:00:00	0
2000-01-01T17:00:00	0
2000-01-01T18:00:00	0
2000-01-01T19:00:00	0
2000-01-01T20:00:00	0
2000-01-01T21:00:00	0
2000-01-01T22:00:00	280
2000-01-01T23:00:00	280
```

Actually, I made a mistake in this second example and have repeated the hour 15 in two ranges.. maybe helpful for diagnostics
@DillonJ DillonJ added bug Something isn't working temporals labels Oct 13, 2021
@manuelma
Copy link
Collaborator

Thank you! I will fix it today.

@manuelma
Copy link
Collaborator

manuelma commented Oct 13, 2021 •
edited
Loading

I believe your expectations are not entirely correct, but I think I know what you mean.

For example in the first example you say you expect 2000-01-01T07:00:00 to yield 280 but that doesn't make sense to me. I believe the rows are a little shifted.

@manuelma
Copy link
Collaborator

I think I know the reason for the misunderstanding. For you, hour 1 corresponds to 00:00 to 01:00. For me, that's hour 0. Hours, minutes, and seconds are zero-based, whereas month and day are one-based (in my book).

I guess it's okey to make everything 1-based. I will go with that.

manuelma added a commit that referenced this issue Oct 13, 2021
@manuelma
Fix time pattern matching at boundaries
0a145a9 
Re #78
@manuelma
Copy link
Collaborator

Something done, please let me know...

@DillonJ
Copy link
Contributor Author

DillonJ commented Oct 13, 2021

I think I know the reason for the misunderstanding. For you, hour 1 corresponds to 00:00 to 01:00. For me, that's hour 0. Hours, minutes, and seconds are zero-based, whereas month and day are one-based (in my book).

I guess it's okey to make everything 1-based. I will go with that.

That may be the reason for part of it, but not for the halving of the values

@manuelma
Copy link
Collaborator

I think I know the reason for the misunderstanding. For you, hour 1 corresponds to 00:00 to 01:00. For me, that's hour 0. Hours, minutes, and seconds are zero-based, whereas month and day are one-based (in my book).
I guess it's okey to make everything 1-based. I will go with that.

That may be the reason for part of it, but not for the halving of the values

That's right, I meant it that way. Both the shifting and the halving should be fixed now.

@DillonJ
Copy link
Contributor Author

DillonJ commented Oct 13, 2021

Nice one, thanks for that.

2000-01-01T00:00:00 0
2000-01-01T01:00:00 0
2000-01-01T02:00:00 0
2000-01-01T03:00:00 0
2000-01-01T04:00:00 0
2000-01-01T05:00:00 0
2000-01-01T06:00:00 0
2000-01-01T07:00:00 280
2000-01-01T08:00:00 280
2000-01-01T09:00:00 280
2000-01-01T10:00:00 280
2000-01-01T11:00:00 280
2000-01-01T12:00:00 280
2000-01-01T13:00:00 280
2000-01-01T14:00:00 280
2000-01-01T15:00:00 280
2000-01-01T16:00:00 280
2000-01-01T17:00:00 0
2000-01-01T18:00:00 0
2000-01-01T19:00:00 0
2000-01-01T20:00:00 0
2000-01-01T21:00:00 0
2000-01-01T22:00:00 0
2000-01-01T23:00:00 0

2000-01-01T00:00:00 280
2000-01-01T01:00:00 280
2000-01-01T02:00:00 280
2000-01-01T03:00:00 0
2000-01-01T04:00:00 0
2000-01-01T05:00:00 0
2000-01-01T06:00:00 0
2000-01-01T07:00:00 0
2000-01-01T08:00:00 0
2000-01-01T09:00:00 0
2000-01-01T10:00:00 280
2000-01-01T11:00:00 280
2000-01-01T12:00:00 280
2000-01-01T13:00:00 280
2000-01-01T14:00:00 280
2000-01-01T15:00:00 0
2000-01-01T16:00:00 0
2000-01-01T17:00:00 0
2000-01-01T18:00:00 0
2000-01-01T19:00:00 0
2000-01-01T20:00:00 0
2000-01-01T21:00:00 0
2000-01-01T22:00:00 280
2000-01-01T23:00:00 280

@manuelma manuelma reopened this Oct 13, 2021
@manuelma
Copy link
Collaborator

manuelma commented Oct 13, 2021 •
edited
Loading

I think before we close, we just need to make a final effort to write in the documentation what we decided here, that all intervals are 1-based and inclusive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working temporals
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@DillonJ @manuelma