-
Notifications
You must be signed in to change notification settings - Fork 11
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
Volume in dclab<=0.36.1 is overestimated by on average 2µm³ #141
Comments
This is an unsettling find. According to https://de.mathworks.com/matlabcentral/fileexchange/36525-volrevolve (which this implementation is based on), this is what happens if you pass a clock-wise contour:
@maikherbig Do you have any insights? |
After comparing with the original matlab script, It also appears as if this line is actually supposed to read v3 = -1 * d_r**2 * z_vec_m1 I'm afraid this calls for thorough testing, starting with these tests in the original script: clear all
R0 = 5 ;
a = 1 ;
npoints = 100 ;
theta = 2*pi*[0:1:npoints-1]'/double(npoints-1) ;
R = R0 + a*cos(theta) ;
Z = a*sin(theta) ;
vol_analytic = 2 * pi^2 * R0 * a^2 ;
% >> 98.6960
vol = volRevolve(R,Z) ;
% >> 98.6298 (6.7e-04 relative error)
% Do it again with npoints = 1000, get:
% >> 98.6954 (6.6e-06 relative error)
% As expected, it's always slightly small because the polygon inscribes the
% circle.
% Verification for washer (rectangular toroid), with the radius of the
% 'hole' in the washer being a, and the outer radius of the washer being b.
% (Thus the width of the metal cross section is b-a.) The height of the
% washer is h. Then the volume is pi * (b^2 - a^2) * h. Run this code:
clear all
a = 1 ;
b = 2 ;
h = 10 ;
R = [a; b; b; a; a] ;
Z = [0; 0; h; h; 0] ;
vol_analytic = pi * (b^2 - a^2) * h ;
% >> 94.2478
vol = volRevolve(R,Z) ;
% >> 94.2478 |
Hi, |
Another issue is this line: After taking a look at the test function, I believe that this is a misinterpretation of what "R" is. It is not the length of the vector (x,z). It is, in fact, x. |
What's also fishy is the |
To answer the original issue, the But, there was still an issue: The lower part of the contour had negative values. As a result that part always returned negative values for the volume - hence the fishy |
Here are the first results using these reference datasets: https://dcor.mpl.mpg.de/dataset/figshare-7771184-v2 This means that on average, the previous implementation overestimated the volume by roughly 2µm³ on average. Here the same plot, normalized by the new volume (roughly 1-1.5 % overestimation in previous implementation on average): |
The method counter_clockwise() in the file dclab/features/volume is supposed to return contour coordinates in a counter-clockwise order, yet the returned coordinates are in clockwise order.
minimal working example:
gives the output:
[ 1 1 -1 -1]
[ 1 -1 -1 1]
The text was updated successfully, but these errors were encountered: