Calculates the Volume of RHC. The worked out Mathematics can be found here. Right now it is in image form. I'll write it down in a better doc format ASAP.
Looks good. Two small issues:
- You should have mentioned that your patch includes a fix for rt_rhc_surf_area() too.
- Add rt_rhc_volume() to table.c so that it can be used.
Regards,
Daniel
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hello Daniel,
My OS had crashed, so I had to install another one. Sorry for the late response.
I haven't done any testing for the rt_rhc_volume() function yet and I haven't thought about it. Do you have any suggestions?
I will update you if I find something interesting :)
Regards,
Kalpit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I tried to test your patch before and after applying your changes. I thought by myself best in mged. Therefore I did a grep for volume in src/libged. So I find the analyze command (analyze.c source file) as a good candidate for testing the area and volume functions. Unfortunately the rhc is still a todo there. But maybe analyze_general() will work with it?
Regards,
Daniel
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
If you search the source tree for "volume", you'd also have found that you can use the gqa command (it samples with rays). Good way to independently test your logic.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi Daniel! Hi Sean!
So, I did the testing and turns out I used both of following the commands :
analyze <object>
gqa -Av -v <object>
The final Volume has different values in both the commands and they are not so close. I hope that's fine because gqa is supposed to be an approximate measure right? Still, it might be that they're too far away -- the values. So, you can take a look at the values obtained here and maybe tell me if those values are alright?
Thanks :)
Regards,
Kalpit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Kalpit, this looks like a fantastic validation. They are close with the tolerance being used. The gqa values are already within 4 digits of precision of your directly computed value (assuming that's what analyze is reporting) and gqa's grid spacing is still pretty coarse. If you bumped up gqa's refinement tolerance, it'd almost certainly converge even more. If there was a problem with the math, I'd expect much bigger differences.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Ups, it doesn't compile:
/home/rossberg/Devel/BRL-CAD/brlcad/src/librt/primitives/rhc/rhc.c: In function ‘rt_rhc_volume’:
/home/rossberg/Devel/BRL-CAD/brlcad/src/librt/primitives/rhc/rhc.c:1821:16: error: unused variable ‘arclen’ [-Werror=unused-variable]
fastf_t A, arclen, integralArea, a, b, magB, sqrt_ra, height;
^
cc1: all warnings being treated as errors
You should reenable BRLCAD_ENABLE_STRICT to see the error too. It's on by default, maybe you have switched it of for some tests.
Daniel
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi Kalpit,
Looks good. Two small issues:
- You should have mentioned that your patch includes a fix for rt_rhc_surf_area() too.
- Add rt_rhc_volume() to table.c so that it can be used.
Regards,
Daniel
Hi Daniel,
Sorry for not mentioning about the changes in rt_rhc_surf_area().
I have added rt_rhc_volume() to table.c. Please find the updated patch attached.
Regards,
Kalpit
Kalpit,
how have you tested the new rt_rhc_volume() function?
Daniel
Hello Daniel,
My OS had crashed, so I had to install another one. Sorry for the late response.
I haven't done any testing for the rt_rhc_volume() function yet and I haven't thought about it. Do you have any suggestions?
I will update you if I find something interesting :)
Regards,
Kalpit
Indeed, I do :)
I tried to test your patch before and after applying your changes. I thought by myself best in mged. Therefore I did a grep for volume in src/libged. So I find the analyze command (analyze.c source file) as a good candidate for testing the area and volume functions. Unfortunately the rhc is still a todo there. But maybe analyze_general() will work with it?
Regards,
Daniel
If you search the source tree for "volume", you'd also have found that you can use the gqa command (it samples with rays). Good way to independently test your logic.
Hi Daniel! Hi Sean!
So, I did the testing and turns out I used both of following the commands :
The final Volume has different values in both the commands and they are not so close. I hope that's fine because gqa is supposed to be an approximate measure right? Still, it might be that they're too far away -- the values. So, you can take a look at the values obtained here and maybe tell me if those values are alright?
Thanks :)
Regards,
Kalpit
Kalpit, this looks like a fantastic validation. They are close with the tolerance being used. The gqa values are already within 4 digits of precision of your directly computed value (assuming that's what analyze is reporting) and gqa's grid spacing is still pretty coarse. If you bumped up gqa's refinement tolerance, it'd almost certainly converge even more. If there was a problem with the math, I'd expect much bigger differences.
Ups, it doesn't compile:
/home/rossberg/Devel/BRL-CAD/brlcad/src/librt/primitives/rhc/rhc.c: In function ‘rt_rhc_volume’:
/home/rossberg/Devel/BRL-CAD/brlcad/src/librt/primitives/rhc/rhc.c:1821:16: error: unused variable ‘arclen’ [-Werror=unused-variable]
fastf_t A, arclen, integralArea, a, b, magB, sqrt_ra, height;
^
cc1: all warnings being treated as errors
You should reenable BRLCAD_ENABLE_STRICT to see the error too. It's on by default, maybe you have switched it of for some tests.
Daniel
Uh-oh. I did come across this error when I was doing the testing but somehow I skipped updating the patch.
Anyways, the updated patch is attached. :)
Regards,
Kalpit
Do you have a patch for analyze.c too?
Hi Daniel,
I have attached the analyze.c patch.
Regards,
Kalpit
Hmm, almost.
First, this patch task is already closed (accepted). I.e. you should open a new one. And, you should fix the todo comment too.
Daniel