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

Issues with banking functions #62

Closed
jrnold opened this issue May 15, 2016 · 1 comment
Closed

Issues with banking functions #62

jrnold opened this issue May 15, 2016 · 1 comment

Comments

@jrnold
Copy link
Owner

jrnold commented May 15, 2016

Issues raised by a user in an email with the functions in banking:

(1) you take the atan() of raw slopes, but in Cleveland (1993) the weighted average absolute orientation criterion is with normalised slopes, that is they are scaled by their range

(2) you calculate the length of line segments as independent of the aspect ratio, but Cleveland (1993) clealry shows the weight as a function of the aspect ratio

(3) why do you optimize the square of the mean (^2) ?

and

explain why you omitted to square the LOR and GOR criteria in bank_slopes_gor and bank_slopes_lor, as shown in Heer and Agrawala (2006)

jrnold added a commit that referenced this issue Jul 11, 2016
I could not get the optimization for ao, lor, and gor to work reliabily.
Since these do not produce results much different than either median or
absolute slope banking, I'm removing them.

This closes #62
@jrnold
Copy link
Owner Author

jrnold commented Jul 11, 2016

  1. I take atan(slopes / alpha) which effectively normalizes them.
  2. That was a mistake - I forgot to normalize them.
  3. That was to take a squared distance between the mean and 45 degrees. I could have used a root finder instead.
  4. It was a mistake to omit those.

After working through the code, I had too many issues getting the various methods involving optimization to find their maxima. I'm removing the methods for lor, gor, and ao. If they are specified, the function will use ms instead with a warning.

@jrnold jrnold closed this as completed Jul 11, 2016
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

No branches or pull requests

1 participant