Skip to content

Commit

Permalink
Merge pull request #42 from Bosma/osmaxpos
Browse files Browse the repository at this point in the history 
osMaxPos rounding to digit parameter, pull request #42
  • Loading branch information
@braverock
braverock committed Jul 9, 2016
2 parents 43ac611 + 788513b commit 559d512
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
6 changes: 3 additions & 3 deletions R/osFUNs.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ getPosLimit <- function(portfolio, symbol, timestamp){
#' @export
#' @note
#' TODO integrate orderqty='all' into osMaxPos for non-risk exit orders by combining side and pos for exits
osMaxPos <- function(data, timestamp, orderqty, ordertype, orderside, portfolio, symbol, ruletype, ...){
osMaxPos <- function(data, timestamp, orderqty, ordertype, orderside, portfolio, symbol, ruletype, digits=0, ...){
# check for current position
pos<-getPosQty(portfolio,symbol,timestamp)
# check against max position
Expand Down Expand Up @@ -177,7 +177,7 @@ osMaxPos <- function(data, timestamp, orderqty, ordertype, orderside, portfolio,
# buy long
if(orderqty>0 && orderside=='long') {
# note no round lots for max clip
clip <- round(PosLimit[,"MaxPos"] / PosLimit[,"LongLevels"], 0)
clip <- round(PosLimit[,"MaxPos"] / PosLimit[,"LongLevels"], digits)

if ((orderqty+pos) > PosLimit[,"MaxPos"]) {
# this order would put us beyond the MaxPos limit
Expand All @@ -203,7 +203,7 @@ osMaxPos <- function(data, timestamp, orderqty, ordertype, orderside, portfolio,
#sell short
if(orderqty<0 && orderside=='short') {
# note no round lots for max clip
clip <- round(PosLimit[,"MinPos"] / PosLimit[,"ShortLevels"], 0)
clip <- round(PosLimit[,"MinPos"] / PosLimit[,"ShortLevels"], digits)

if ((orderqty+pos) < PosLimit[,"MinPos"]) {
# this order would put us beyond the MinPos limit
Expand Down
2 changes: 2 additions & 0 deletions man/osMaxPos.Rd
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 comments on commit 559d512

@evelynmitchell
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The argument to round() is for the number of digits after the decimal place to keep, not the number of digits to round to.

round(400.04, 0) = 400
numeric number of digits (0) to round positions size 400.04 results in ''

round(400.04, 3) = 400.04
numeric number of digits (3) to round positions size 400.04 results in 400

Is that description what you intended as the behavior?

@joshuaulrich
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I updated the docs this PR/merge touched, I did not attempt to alter the intent of the parameter documentation. That is to say, this is still a potential issue.

@braverock
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree that the documentation could be improved. It should probably be drawn from and refer to round, e.g.:

#' @param digits integer indicating the number of decimal places to be used. Negative values are allowed. See \code{\link[base]{round}}

Please sign in to comment.