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

table.DownsideRiskRatio/table.Variability variable freq not defined #128

Open
ThomasHehehehe opened this issue Jul 11, 2019 · 8 comments · May be fixed by #164
Open

table.DownsideRiskRatio/table.Variability variable freq not defined #128

ThomasHehehehe opened this issue Jul 11, 2019 · 8 comments · May be fixed by #164

Comments

@ThomasHehehehe
Copy link

Hello,
in table.DownsideRiskRatio function:
freq is not defined if scale is not NA.

y = checkData(R)
columns = ncol(y)
columnnames = colnames(y)
if (is.na(scale)) {
freq = periodicity(R)
...
}
for (column in 1:columns) {
...
znames = c(paste0(**freq$**scale, " downside risk"), "Annualised downside risk",
"Downside potential", "Omega", "Sortino ratio",
"Upside potential", "Upside potential ratio", "Omega-sharpe ratio")
...
}

@braverock
Copy link
Owner

thanks for the report.

Could you describe why you think this is a problem? If you don't pass scale, the function tries to figure it out.

Do you have an example of a problem you're having?

I'm happy to take a look, but I want to know what you were expecting, and what, if anything, is not working.

@ThomasHehehehe
Copy link
Author

Thank you for quick response!

table.DownsideRiskRatio(port, scale = 252, digits=4)
does not work
while
table.DownsideRiskRatio(port, digits=4)
works.

I checked the function and found that if scale is defined,
and the code dose not go into if{} ,
therefore freq will not be defined.
However, freq is used anyway in for{} in following line:
znames = c(paste0(freq$scale, " downside risk"), "Annualised downside risk",

Thank you! PerformanceAnalytics is a very nice package.

@ThomasHehehehe
Copy link
Author

version: PerformanceAnalytics_1.5.3

@braverock
Copy link
Owner

upon investigation, your report doesn't make sense to me, and I still dont have a reproducible example.

The code in question is this:

    if(is.na(scale)) {
        freq = periodicity(y)
        switch(freq$scale,
            minute = {stop("Data periodicity too high")},
            hourly = {stop("Data periodicity too high")},
            daily = {scale = 252},
            weekly = {scale = 52},
            monthly = {scale = 12},
            quarterly = {scale = 4},
            yearly = {scale = 1}
        )
    }

so freq is defined if scale is not defined, and it is only used inside that if block, not any where else in the function.

In all cases, scale will be defined, and scale is what is used later in the function.

I suspect that your port object is non-conforming in some way, but without a reproducible example demonstrating the problem you say you're having, I can't demonstrate that.

@ThomasHehehehe
Copy link
Author

Hi! Thanks for following up!
Actually, in the script for table.DownsideRiskRatio(),
freq is used outside the if block.

for(column in 1:columns) {
z = c(DownsideDeviation(y[,column,drop=FALSE], MAR = MAR), DownsideDeviation(y[,column,drop=FALSE], MAR = MAR)*sqrt(scale), DownsidePotential(y[,column,drop=FALSE], MAR = MAR), UpsideRisk(y[,column,drop=FALSE], MAR = MAR, stat = "potential")/DownsidePotential(y[,column,drop=FALSE], MAR = MAR), SortinoRatio(y[,column,drop=FALSE], MAR = MAR), UpsideRisk(y[,column,drop=FALSE], MAR = MAR, stat = "potential"), UpsidePotentialRatio(y[,column,drop=FALSE], MAR = MAR), OmegaSharpeRatio(y[,column,drop=FALSE], MAR = MAR))
znames = c(paste0(freq$scale," downside risk"), "Annualised downside risk", "Downside potential", "Omega", "Sortino ratio", "Upside potential", "Upside potential ratio", "Omega-sharpe ratio")
if(column == 1) {
resultingtable = data.frame(Value = z, row.names = znames)
}
else {
nextcolumn = data.frame(Value = z, row.names = znames)
resultingtable = cbind(resultingtable, nextcolumn)
}
}

In line 64, freq$scale is used.
This causes the bug if scale is well defined in the parameter input.
Hope I make myself clear.
Thank you!

@braverock
Copy link
Owner

OK, now I understand you. I will fix.

@ThomasHehehehe
Copy link
Author

OK, now I understand you. I will fix.

Thank you! Similar issues may also happen in other functions using scale input.

@DzimitryM
Copy link

Please fix the same bug in function 'table.Distributions()'

It can be done by just placing the following string outside the block 'if(is.na(scale)) {...}'
freq = periodicity(y)

domodwyer added a commit to domodwyer/PerformanceAnalytics that referenced this issue Jul 5, 2021
This commit removes references to 'freq' which is not assigned when
the argument 'scale' != NA for the following tables:

	* table.Distributions
	* table.DownsideRiskRatio
	* table.Variability

Fixes braverock#128.
@domodwyer domodwyer linked a pull request Jul 5, 2021 that will close this issue
domodwyer added a commit to domodwyer/PerformanceAnalytics that referenced this issue Jul 5, 2021
This commit removes references to 'freq' which is not assigned when
the argument 'scale' != NA for the following tables:

	* table.Distributions
	* table.DownsideRiskRatio
	* table.Variability

Fixes braverock#128.
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 a pull request may close this issue.

3 participants