[ENH] circvar calculated simply as 1-R - #5747

This is not backwards compatible, right? Wouldn't adding an option be a better idea?

No, it's not compatible. The circvar method used until now, takes simply circstd squared (before the normalization). However, I haven't seen yet such a definition anywhere else. The sources I've looked in are:

• wikipedia
• Matlab's Circular Statistics Toolbox
• R's Circular Statistics Toolbox
• Fisher NI, Statistical analysis of circular data. Cambridge, 1993.
• Jammalamadaka SR and SenGupta A, Topics in circular statistics, Singapore, 2001.

All of them define the circular variance as 1-R. Only the last reference defines it as n(1-R), where n is the array size. Therefore, the circvar definition used until now might not be correct.

Is a compatibility option still needed in this case?

@@            master   #5747   diff @@
======================================
  Files          235     235       
  Stmts        43219   43225     +6
  Branches      8143    8145     +2
  Methods          0       0       
======================================
+ Hit          33601   33605     +4
- Partial       2591    2593     +2
  Missed        7027    7027       

Review entire Coverage Diff as of 2a6daa7

Powered by Codecov. Updated on successful CI builds.

How compatible (conceptually) circvar and circstd are going to be then?

I don't remember any details for how transformation of circular data works.
What looks strange to me in this change is that the variance becomes independent of the units. This is usually not the case with any other random variables. In the case of 360 degrees, it looks strange having a tiny variance, as in the changed test case.

If this is just a renormalization of the variance into radians, then this should be an option not a backwards incompatible redefinition.

mirca said:

How compatible (conceptually) circvar and circstd are going to be then?

circstd and circvar are not going to be conceptually compatible. Fisher, 1993 is explicit that circular std is not defined as circular var**0.5.

josef-pkt said:

What looks strange to me in this change is that the variance becomes independent of the units. This is usually not the case with any other random variables.

This is how circular variance is defined in the literature, with values between 0 and 1, and independent on the units.

If this is just a renormalization of the variance into radians, then this should be an option not a backwards incompatible redefinition.

It's not only a renormalization. After normalisation, the PR's circvar is approximately twice smaller than the linear variance of data with very small deviations around the mean. For big variances, however, I would expect the old and new definition to deviate even after normalisation.

I would strongly suggest to keep the new definition of circvar. However, an optional variable could set whether we expect 1-R as output or the de-normalised value ((high-low)/(2*pi))**2*(1-R). The same optional variable should also be implemented for the (de)normalisation of cirstd.

3 years ago I was working on a circular "toolbox" based initially on a matlab translation.

I used var = 1 - R, and a wrapper that converts between the circle units (degrees, hours or whatever) into radians and back.
I don't remember much of the details but was reading mainly Mardia's book for this.

AFAIR (!), I never managed to figure out these scipy stats function and left them to Travis. (I thought differences might be due to scaling. There are also weird things in R packages, where I wasn't quite sure where their scaling or transformation comes from.)

A new push:

• added an optional parameter for result normalization to circstd and circvar. Because of compatibility, the returned values are scaled by default depending on the variable range as it was until now.
• docstrins of circmean, circstd and circvar have a more accurate description of the high and low parameters.

I gather there might be some use of / experience with these sorts of functions in astropy land.

@ev-br You are right. See #4472.

+1 for this feature. Also, it might be also good to allow a weights argument.

@kleskjr I'd like to help you finish this up. Can you select "Allow edits from maintainers"?

@kleskjr Thought I'd check in again. If you could allow select "Allow edits from maintainers", I'd appreciate it!

@mdhaber, edits are allowed. Sorry for the delay!

Thanks @kleskjr. How important do you think the normalize option is for circvar?

In cirstd, I think I see how having low and high could be useful with and without normalize:

• whatever your units of angle were originally (e.g. degrees), you can use low and high to convert them to the range 0 to 2 pi.
• Since the output can be interpreted as being in units of radians, normalize gives you the option to convert back to the original units. This is supported by the literature - at least Mardia (see DOC: stats.circstd: add reference, notes, comments #15652).

The validity of normalize for circvar is less clear to me. The output is not in radians; it is in the range of 0 to 1, and we can think of it as the distance from the outside of a unit circle, right? If that's correct, it seems that the more appropriate normalization would be in terms of the radius of the original circle, but we don't have that information. I don't see support for this sort of thing in the literature, so I think it might be easier to merge this without normalize, at least in circvar.

@steppi I suspect that the reason this hasn't already been fixed is that others are unsure of the math. That's why I tagged you, but NP if it's not of interest.

@steppi I suspect that the reason this hasn't already been fixed is that others are unsure of the math. That's why I tagged you, but NP if it's not of interest.

Definitely of interest but my time is scarce now. I think I can take a look in 2-3 weeks.

I don't see support for this sort of thing in the literature, so I think it might be easier to merge this without normalize, at least in circvar.

@mdhaber I concur.

Thanks @rkern. Other than the adjustment above, this is ready from my perspective. If you and @steppi approve the PR and there are no concerns from the mailing list, I'll go ahead and merge it in a week.

There were no replies to the mailing list post, there is +1 explicit approval from @steppi (and me), and there seems to be other support above, so I'm merging. I added a note about the backward-incompatible change in the release notes. Thanks for the review @steppi!