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

The duration filed on the Method class is incorrectly reported in summary #9

Closed
mihaiconstantin opened this issue Apr 29, 2022 · 0 comments · Fixed by #15
Closed

The duration filed on the Method class is incorrectly reported in summary #9

mihaiconstantin opened this issue Apr 29, 2022 · 0 comments · Fixed by #15
Assignees
Labels
bug Something isn't working

Comments

@mihaiconstantin
Copy link
Owner

mihaiconstantin commented Apr 29, 2022

The duration field of the Method class is determined based on

powerly/R/Method.R

Lines 172 to 179 in d6be4e6

# Start the clock.
private$.start_time <- Sys.time()
# Run the method.
private$.run(replications, monotone, increasing, df, solver_type, boots, lower_ci, upper_ci)
# Stop the clock.
private$.end_time <- Sys.time()

as

powerly/R/Method.R

Lines 184 to 185 in d6be4e6

# Time elapsed until converged.
duration = function() { return(private$.end_time - private$.start_time) },

and then reported in the summary as follows:

powerly/R/Method.R

Lines 210 to 211 in d6be4e6

summary.Method <- function(object, ...) {
cat("\n", "Method run completed (", as.numeric(round(object$duration, 4)), " sec):", sep = "")

When the method takes more the 60 seconds, the units of the duration field change from seconds to minutes, courtesy of the default difftime function. For example:

start <- Sys.time() - 61
end <- Sys.time()

print(start)
# "2022-04-29 15:22:52 CEST"

print(end)
# "2022-04-29 15:23:53 CEST"

duration <- end - start
cat("Method run completed (", as.numeric(round(duration, 4)), " sec)", sep = "")
# Method run completed (1.0194 sec)

I think it's better to just use the same unit everywhere, e.g.:

duration <- difftime(end, start, units = "secs")
cat("Method run completed (", as.numeric(round(duration, 4)), " sec)", sep = "")
# Method run completed (61.1642 sec)

This applies to other classes that have duration fields as well.

@mihaiconstantin mihaiconstantin self-assigned this Apr 29, 2022
@mihaiconstantin mihaiconstantin added the bug Something isn't working label Apr 29, 2022
@mihaiconstantin mihaiconstantin changed the title The duration filed on the Method class is incorrectly reported The duration filed on the Method class is incorrectly reported in summary Apr 29, 2022
Repository owner moved this from Todo to Done in powerly Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant