Skip to content

removed useless reinitialisation of time grid (thanks to @amaggiulli) - #93

Closed
klausspanderen wants to merge 1 commit into
lballabio:masterfrom
klausspanderen:master
Closed

removed useless reinitialisation of time grid (thanks to @amaggiulli)#93
klausspanderen wants to merge 1 commit into
lballabio:masterfrom
klausspanderen:master

Conversation

@klausspanderen

Copy link
Copy Markdown
Contributor

No description provided.

@igitur

igitur commented May 20, 2016

Copy link
Copy Markdown
Contributor

Credit to @amaggiulli for spotting it.

@klausspanderen klausspanderen changed the title removed useless reinitialisation of time grid (thanks to Francois Botha) removed useless reinitialisation of time grid (thanks to @amaggiulli) May 20, 2016
@klausspanderen

Copy link
Copy Markdown
Contributor Author

thanks, I've changed it.

On Friday, May 20, 2016 01:02:25 PM Francois Botha wrote:

Credit to @amaggiulli[1] for spotting it.
—You are receiving this because you authored the thread.Reply to this email directly or
view it on GitHub[2][3]


[1] https://github.com/amaggiulli
[2] #93 (comment)
[3]
https://github.com/notifications/beacon/AEeY4L_x9oHOENvYBxGs17B6cQniLvu0ks5q
DhNRgaJpZM4IjgSn.gif

@igitur

igitur commented May 23, 2016

Copy link
Copy Markdown
Contributor

Please also have a look at #92 . That's @amaggiulli 's alternative solution. I'm just curious why it's possible to remove all those variables and still have the tests pass. Aren't they used?

@pcaspers

Copy link
Copy Markdown
Contributor

I'd say one grid is 0, 1, 2, 3, 4 ... and the other (the "correct" one) is slightly different at some points (differing maybe by +-1/365, +-2/365 or the like), but this is swallowed by the MC tolerance at the end.

@klausspanderen

Copy link
Copy Markdown
Contributor Author

Let's close this PR as we have alternative solutions in the line

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants