Screenshot instructions:
Windows
Mac
Red Hat Linux
Ubuntu
Click URL instructions:
Rightclick on ad, choose "Copy Link", then paste here →
(This may not be possible with some types of ads)
From: Dawn Thomas <thedawnthomas@gm...>  20080420 14:37:29

Hi all, This is regarding the following item on TODO * change root solver to have coefficients in the right order (!) Please see patch 1947026 at SF. I was wondering about the following aspects. 1. Is there any particular reason for the existence of roots.c under librt ? Should we not transfer all the code to libbn ? 2. Is there any thought about the destruction of the polynomial by rt_poly_roots as mentioned in the comments ? Should we be trying to preserve the polynomial or has this been a design decision taking into account some consideration that polynomial preservation as such is not necessary ? 3. The internal polynomial root solution methods ( eg. rt_poly_eval_w_2derivatives , rt_poly_checkroots , rt_poly_deflate ) currently expects the coefficients in the present order. Would we be just shifting the inversion code finally to libbn ( after librt interface deprecation ) or recoding such methods to follow the new representation? I believe considering the fact that most of such internal methods appear more logical in the present state it would be better to not recode and just transfer the inversion function. Also, on a connected note, should we recode, since only some of the polynomial operations defined in libbn/poly.c ( bn_poly_mul , bn_poly_scale ) are symmetrical or independent of order of coefficents, we will have to recode the others ( bn_poly_add , bn_poly_sub , bn_poly_synthetic_division , bn_poly_quadratic_roots, bn_poly_cubic_roots, bn_poly_quartic_roots ) to follow the proposed convention. Sincerely yours,  Dawn Thomas IIT Kharagpur " the strength to change what i can, the inability to accept what i can't, and the incapacity to tell the difference" 
From: Dawn Thomas <thedawnthomas@gm...>  20080423 18:08:01

2008/4/21 Christopher Morrison <brlcad@...>: > Hey Dawn, great questions... > > > >1. Is there any particular reason for the existence of roots.c under > >librt ? Should we not transfer all the code to libbn ? > > There was a reason, but it doesn't matter. It can and should move, deprecating the current rt_poly_*() routines. > > I have transfered all the functions from librt/roots.c to libbn/poly.c except for rt_poly_roots() which is the only function called externally. Since all the other rt_poly_*() functions are called only internally renaming them to bn_poly_*() shouldnt produce any changes. I shall get started on validation tests once my thesis finishes up ( May 10) . Suggestions welcome in the meantime :) > >2. Is there any thought about the destruction of the polynomial by > >rt_poly_roots as mentioned in the comments ? Should we be trying to > >preserve the polynomial or has this been a design decision taking into > >account some consideration that polynomial preservation as such is not > >necessary ? > > I think it just happened that way. It would be nice to have the parameters be const given there's no significant value in what results. > this would basically only require recoding of rt_poly function since it is the one who modifies the polynomial and passes it on to other functions to modify it further. > > >3. The internal polynomial root solution methods ( eg. > >rt_poly_eval_w_2derivatives , rt_poly_checkroots , rt_poly_deflate ) > >currently expects the coefficients in the present order. Would we be > >just shifting the inversion code finally to libbn ( after librt > >interface deprecation ) or recoding such methods to follow the new > >representation? I believe considering the fact that most of such > >internal methods appear more logical in the present state it would be > >better to not recode and just transfer the inversion function. > > As public functions (which they presently are), they should be consistent. That is, the bn_poly_t type needs to be the same everywhere with the coefficients having the same meaning otherwise we're just asking for really obscure problems down the road. The *only* reason to have a rt_* function that does an inversion would be to allow for deprecation notification. > So fundamentally to maintain the constancy of bn_poly_t types meaning, we would need to recode the functions which implicitly assumes as particular order of coefficients ( rt_poly_eval_w_2derivatives , rt_poly_checkroots , rt_poly_deflate as well as bn_poly_add, bn_poly_sub , bn_poly_synthetic_division , bn_poly_quadratic_roots, bn_poly_cubic_roots, bn_poly_quartic_roots ) > > >Also, on a connected note, should we recode, since only some of the > >polynomial operations defined in libbn/poly.c ( bn_poly_mul , > >bn_poly_scale ) are symmetrical or independent of order of > >coefficents, we will have to recode the others ( bn_poly_add , > >bn_poly_sub , bn_poly_synthetic_division , bn_poly_quadratic_roots, > >bn_poly_cubic_roots, bn_poly_quartic_roots ) to follow the proposed > >convention. > > Yep. There's a bit of work involved to make the enduser API clean from where it presently is. Some of the functions become simplified, some more complicated. I performed the conversion several years ago but didn't have a sufficient breadth of test cases to make me comfortable with the change, so it was shelved/ditched (only a days effort). > > In addition to the retooling, there needs to be a handful of validation tests that exercises the routines affected since this potentially affects analytic raytrace results if something goes wrong. > > Cheers! > Sean > > >  Dawn Thomas IIT Kharagpur " the strength to change what i can, the inability to accept what i can't, and the incapacity to tell the difference" PS: Is JRA around ? am quite eager to meet my mentor. Also i will elaborate the parametric proposal further and define it more clearly on the wiki hopefully this weekend.  Dawn Thomas IIT Kharagpur " the strength to change what i can, the inability to accept what i can't, and the incapacity to tell the difference" 
From: Christopher Sean Morrison <brlcad@ma...>  20080424 02:50:12

On Apr 23, 2008, at 2:06 PM, Dawn Thomas wrote: > I have transfered all the functions from librt/roots.c to libbn/poly.c > except for rt_poly_roots() which is the only function called > externally. Since all the other rt_poly_*() functions are called only > internally renaming them to bn_poly_*() shouldnt produce any changes. > I shall get started on validation tests once my thesis finishes up ( > May 10) . Suggestions welcome in the meantime :) We may only use the one function, but it's unknown whether there are external users that are relying on that functionality. All that means is that those four or so functions will need to flip the order of the coefficients before calling the (new) bn_poly*() functions. We have an API deprecation process documented in doc/deprecation.txt that will be used to make the rt_poly*() functions obsolete. Only suggestion would be to keep the patches flowing while you're working on this  small succinct patches to the patches tracker that can be easily reviewed and applied. After a few issueless patches are received that show coding competency and respect/understanding of the HACKING guidelines, commit access can be enabled. I'm hoping all the students can be at/past that point before GSoC coding begins so that everyone can be collaboratively working directly on trunk. One of my goals for all GSoC participants is to make sure everyone learns to coordinate with our existing devs and to eventually have you operating like any other developer on the project. > this would basically only require recoding of rt_poly function since > it is the one who modifies the polynomial and passes it on to other > functions to modify it further. As well as the others, only because they are public via raytrace.h. When they get moved to libbn, they should probably be marked static if there's no good reason to export them. > So fundamentally to maintain the constancy of bn_poly_t types meaning, > we would need to recode the functions which implicitly assumes as > particular order of coefficients ( rt_poly_eval_w_2derivatives , > rt_poly_checkroots , rt_poly_deflate as well as bn_poly_add, > bn_poly_sub , bn_poly_synthetic_division , bn_poly_quadratic_roots, > bn_poly_cubic_roots, bn_poly_quartic_roots ) Yep. 
Sign up for the SourceForge newsletter:
No, thanks