Skip to content

Conversation

@kiwimic
Copy link

@kiwimic kiwimic commented Oct 24, 2018

I just got to know your package, I was looking for an implementation in R MUS algorithm (especially MUS.factor (confidence.level, pct.ratio)), when I used MUS.factor (confidence.level, pct.ratio) I came across a strange result (practically always trying was 1). When I examined the function, it turned out that there are missing parentheses to divide)

library(MUS)


MUS.calc.n.conservative_VERSION_FROM_PACKAGE_CRAN_AND_GITHUB <- function(confidence.level, tolerable.error, expected.error, book.value) {
  # calculate n consevatively, as per AICPA audit guide
  pct.ratio = expected.error / tolerable.error
  conf.factor = ceiling(MUS.factor(confidence.level, pct.ratio)*100)/100
  ceiling(conf.factor / tolerable.error / book.value)
}

MUS.calc.n.conservative_VERSION_AFTER_CHANGE <- function(confidence.level, tolerable.error, expected.error, book.value) {
  # calculate n consevatively, as per AICPA audit guide
  pct.ratio = expected.error / tolerable.error
  conf.factor = ceiling(MUS.factor(confidence.level, pct.ratio)*100)/100
  ceiling(conf.factor / (tolerable.error / book.value)) ## Added brackets
}



confidence.level <- 0.95
tolerable.error <- 9 * 1000 * 1000
expected.error <- 3 * 1000 * 1000
book.value = 300 * 1000 * 1000`

## 1. Version from package ####
MUS.calc.n.conservative_VERSION_FROM_PACKAGE_CRAN_AND_GITHUB(confidence.level, tolerable.error, expected.error, book.value)
# result = 1 (BAD)
MUS::MUS.calc.n.conservative(confidence.level, tolerable.error, expected.error,book.value)
# result = 1 (BAD)
## 2. Version with updated brackets
MUS.calc.n.conservative_VERSION_AFTER_CHANGE(confidence.level, tolerable.error, expected.error, book.value)
# result = 220 (GOOD)

@gaborcsardi
Copy link

Hi, this is a read-only mirror of CRAN, please see the package authors in DESCRIPTION.

@kiwimic
Copy link
Author

kiwimic commented Oct 24, 2018 via email

@gaborcsardi
Copy link

@kiwimic That's because they forked that repository from here. That is somewhat unfortunate, and does not seem like a good idea.

@kiwimic
Copy link
Author

kiwimic commented Oct 24, 2018

Ok, so there is a away to move this pull request to alsguimaraes/MUS?

@gaborcsardi
Copy link

No, I don't think so. Looks like you were editing alsguimaraes:master, so this code is already in there and no pull request is needed?

@kiwimic
Copy link
Author

kiwimic commented Oct 24, 2018

This is also my first pull request, but when I inspect cran package, and alsguimaraes/MUS on github there is still bug in code. And now when I inspect this pull request I also see that "kiwimic wants to merge 25 commits into cran:master from alsguimaraes:master" which i don't want to.. i want to just add this comment (pull request) and wanted to create single commit after

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