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

Properly enforce timelimits >= 60s #366

Merged
merged 1 commit into from
May 12, 2020
Merged

Properly enforce timelimits >= 60s #366

merged 1 commit into from
May 12, 2020

Conversation

trestletech
Copy link
Contributor

If an exercise had an assigned timelimit of >= 60s, it would never be reaped. This is because by default when you subtract times it becomes a difftime object which will be presented in arbitrary units. Around the 60s mark, you can see we switch from:

> str(Sys.time() - start_time)
 'difftime' num 58.2525360584259
 - attr(*, "units")= chr "secs"

to:

> str(Sys.time() - start_time)
 'difftime' num 1.01057458321253
 - attr(*, "units")= chr "mins"

When you compare on this time, it compares 1 to your assigned time limit and permits it to continue computing.

We need to force the time comparison into seconds in order to accurately compare to our time limit.

If an exercise had an assigned timelimit of >= 60s, it would never be reaped. This is because by default when you subtract times it becomes a `difftime` object which will be presented in arbitrary units. Around the 60s mark, you can see we switch from:

> str(Sys.time() - start_time)
 'difftime' num 58.2525360584259
 - attr(*, "units")= chr "secs"

to:

> str(Sys.time() - start_time)
 'difftime' num 1.01057458321253
 - attr(*, "units")= chr "mins"

When you compare on this time, it compares 1 to your assigned time limit and permits it to continue computing.

We need to force the time comparison into seconds in order to accurately compare to our time limit.
@trestletech trestletech merged commit 1688820 into master May 12, 2020
@trestletech trestletech deleted the time-limit branch May 12, 2020 16:48
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.

2 participants