Le 20/10/2014 16:00, Hank Grabowski a écrit : > I have some time this week to try to get these changes made to the > interpolators. I don't want to do anything without consensus however. So > to try to incorporate the discussion above plus a concerned raised over the > weekend on the JIRA thread I propose: > > 1. Adding the original functionality back so that the original classes are > there, but adding deprecation and very explicit warnings to the > documentation that they should not be used. One release ahead of this I > think they should be removed however. > 2. Move my code into a new PiecewiseBicubicSpline classes. > > The changes I discussed in the e-mail that Giles referred to above I will > be making to these new PiecewiseBicubicSpline classes, and the > TricubicSpline i will do the same Piecewise surname rather than replacing > the classes. > > I do not want to do this with backing outt the existing pull request. I > will simply make the changes and then initiate a follow-up one. > > How does that sound to everyone? We need to get accurate interpolators in > the next release, so the sooner I can squeeze this into my schedule to get > this committed the better.
It sounds good to me, but I prefer to wait for Gilles analysis as he is more aware than me of any problems on this topic. best regards, Luc > > > > On Fri, Oct 17, 2014 at 4:12 PM, Hank Grabowski <h...@applieddefense.com> > wrote: > >> I didn't want to address the situation in my original response since I was >> on a smart phone, a bit torqued up by the original e-mail and I didn't want >> to further agitate the situation by addressing the original >> implementation. Since it seems that's all happened anyway, if you still >> want my newbie two cents then it is the follow. I do not believe that we >> should create identical sounding non-deprecated classes so that we can keep >> the original implementations in their place. These existing classes have >> the appropriate name and calling signature. These existing classes are >> being used by libraries and are returning incorrect results. That's how >> all this was brought to my attention anyway. >> >> If we want to undo changes to BicubicSplineInterpolatingFunction and >> return it back to its original state and move my code into something like >> PieceWiseBicubicSplineInterpolatingFunction then I can see how that would >> be reasonable, but that shouldn't be accessed anywhere in the >> BicubicSplineInterpolator code. The original >> BicubicSplineInterpolatingFunction has trouble with interpolation of even >> planar functions. I'm therefore reluctant to keep that as an option that >> someone could casually invoke in their code, even with deprecation >> warnings. That's the only reason why I removed it in the first place (the >> same discussion is going to happen on the TricubicInterpolation btw). If >> we decide to go that route I would prefer to do that as a new change and >> pull request rather than undo the original pull request. I can create a new >> JIRA incident to capture that. >> >> As to the future changes, my original thought was that as I implement the >> additional algorithms, specifically the B-Spline and regular cubic, that >> there may be some refactoring. I'm not sure if I see a value in an abstract >> base class across all of them. We already have the *variateInterpolator >> interfaces which I think capture the generic nature of the usage of the >> interpolation functions. If I found multiple interpolators using common >> construction code then I would have suggested a refactoring to either a >> factory class or an abstract base class. It isn't apparent to me that >> either of those two will come to fruition. As a point of reference, the two >> interpolators that would be the most same out of the list I had suggested >> were the existing SplineInterpolator and AkimaSplineInterpolator. Their >> algorithms are different to the point where no common code exists between >> them. I have written, and it is my intention to continue writing, unit >> tests around the interpolating functions that do bounds checks and >> numerical accuracy tests that are benchmarked against Monte Carlo runs of >> the Octave interpolators of the same kind. A refactoring iff we need it >> would therefore make sense to me. If you see something that jumps out as >> screaming for a common base class then I'm open to that suggestion. I'm >> just not seeing it right now. >> >> >> >> >> >> On Fri, Oct 17, 2014 at 1:40 PM, Phil Steitz <phil.ste...@gmail.com> >> wrote: >> >>> Lets calm down, guys. As Luc said, we are experimenting here, >>> getting the git workflow established. No one is trying to exclude >>> or discourage anyone. Hank or Luc, can you respond to Gilles' >>> questions about the commit? If not, it should be reverted, but >>> hopefully you can all three agree on a way forward. >>> >>> Phil >>> >>> >>> On 10/17/14 8:31 AM, Hank Grabowski wrote: >>>> Gilles, >>>> >>>> This is the original changes to get the bicubic spline working. These >>> were >>>> originally committed as a diff that was attached to the JIRA incident. >>> The >>>> suggestions in your email were in response to my questions about work >>>> carrying forward from that point. >>>> >>>> I have been very explicit and verbose on what I was doing throughput the >>>> development of those upgrades, both within the JIRA incident and within >>> the >>>> forums. I attempted to incorporate all the comments that I received. I >>>> submitted my code in a way that could have been reviewed. If that isn't >>>> clear then I apologize, however I don't appreciate the connotation that >>>> these changes were done willy nilly or in a rogue fashion. >>>> >>>> Because I want my future contributions to be appreciated and not >>> disrupted >>>> I would like to know how to do this process better/differently. I don't >>>> intend to put substantial effort into development and communication to >>> have >>>> yet another reaction like this. It is as or more frustrating to me as >>> it >>>> appears it is for you. >>>> >>>> Sent from my Android phone >>>> On Oct 17, 2014 10:24 AM, "Gilles" <gil...@harfang.homelinux.org> >>> wrote: >>>> >>>>> Hi. >>>>> >>>>> On Fri, 17 Oct 2014 10:46:53 +0200, Luc Maisonobe wrote: >>>>> >>>>>> Hi Hank, >>>>>> >>>>>> Le 16/10/2014 20:20, Hank Grabowski a écrit : >>>>>> >>>>>>> OK. I submitted the pull request yesterday. I'm going to now >>> remove the >>>>>>> diff from JIRA. >>>>>>> >>>>>>> https://github.com/apache/commons-math/pull/2 >>>>>>> >>>>>> Thank you. I have merged this request and pushed the result to our >>> main >>>>>> repository. The only changes I introduced were fixing end of lines in >>>>>> the new Akima spline files (main and test). Perhaps you should check >>> the >>>>>> git setting core.autocrlf on your side. >>>>>> >>>>>> >>>>>> It seems to me this pull request did not make it to our dev list. Did >>> I >>>>>> simply miss it or is there a problem in the GitHub setting since we >>>>>> updated our repo? Did someone else see the request? If nobody saw it, >>> I >>>>>> think we should ask infra to fix the settings. >>>>>> >>>>>> >>>>> I didn't see the request. >>>>> >>>>> I also did not see the changes before they were committed.[1] >>>>> >>>>> I have no problem with the principle of dropping broken code; but I >>> have >>>>> one with figuring out when it is okay to do so without notice, ignoring >>>>> that care be taken with such changes. >>>>> >>>>> I had suggested to not touch the existing classes and that they should >>>>> be first deprecated, and then removed. Since several alternatives for >>>>> implementing the functionality were proposed, it would have been >>> sensible >>>>> to have an agreement on how to fit them within the library (for >>> example: >>>>> an abstract base class and concrete subclasses for each kind of >>> spline). >>>>> >>>>> In CM, we've had, on one hand, small, trivial, changes that generated a >>>>> lot of unwarranted heat and stalled for days or weeks. And on the >>> other, >>>>> here is an example where big changes are pushed without a discussion. >>>>> >>>>> When I dare to make a suggestion about something,[2] it means that >>>>> I took some time to think about the proposal; the minimum of respect >>>>> for this commitment is to acknowledge the existence of such comments >>>>> and provide an explanation as to why it is better to not follow the >>>>> suggested path: >>>>> >>>>> http://markmail.org/message/tjengf3t6j3hqyph >>>>> >>>>> [If alternative views are really so different that a compromise cannot >>>>> be reached, it is quite simple to count the people who have expressed >>>>> their preference from a list of alternatives (as Phil often posts). >>>>> In this instance, only I have expressed my preference; hence I do not >>>>> understand why something else has been committed.] >>>>> >>>>> My opinion is that we should have created new classes containing the >>>>> working implementation(s) of the interpolation, and deprecated but >>>>> kept the old ones at least up to release 4.0, advertizing (in the >>>>> release notes and in the Javadoc) that they are not working properly >>>>> (although they follow reference "such and such"). [Someone might >>>>> have used that window of opportunity to point to the root cause of >>>>> the issue.] >>>>> >>>>> So, was there a problem with that approach? >>>>> >>>>> I'm sorry if this naive questioning looks trivial to some of you, >>>>> but I'd honestly like to know if this project is team work, and how >>>>> it's supposed to work in practice. >>>>> >>>>> I'm also sorry if this rant has been caused by a simple overlook >>>>> of the post I'm referring to above. However even if it's the case, >>>>> there is a problem. >>>>> >>>>> I hope I'm not being misunderstood[3]: it is great that Hank >>>>> could fix CM's spline interpolators. >>>>> In this opinion, I'm only concerned with the overall aspect of >>>>> contributing to a project that purports to be more that a bunch >>>>> of hooks to math functions, and about the design of which people >>>>> who have been contributing for some time might have earned (?) >>>>> the right to be listened to.[4] >>>>> >>>>> >>>>> Regards, >>>>> Gilles >>>>> >>>>> [1] And I'm also not yet comfortable with looking at large changes >>>>> due to my surely inefficient handling of "git"... >>>>> [2] This is already after the self-censorship filter, on issues >>>>> where I know in advance that challenging the adopted view will >>>>> either be ignored or go nowhere... :-} >>>>> [3] As is often the case by people who do not carefully read what >>>>> I write in this forum. >>>>> [4] Which, I know, is not the same as being heard, and even less >>>>> being agreed with. ;-) >>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>> >>>>> >>> >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >>> >> > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
