Le 29/01/2011 05:31, Gilles Sadowski a écrit : > Hello. Hi Gilles,
> >>>>>>> OK. But now that we have detected an "aroma" around unilaterally >>>>>>> making UnivariateRealFunction throw Math*User*Exception, I wonder if >>>>>>> there is a way to introduce an unchecked parent that gets us out of >>>>>>> this. We may want to reserve the right to do this in 3.0, so the "head >>>>>>> start" in 2.2 might be a head start to nowhere, unless we find a way >>>>>>> to fix it in 2.2 (or you convince us that the current setup is an OK >>>>>>> long-term solution). >>>>>> >>>>>> I don't understand what you mean. >>>>>> "MathUserException" is unchecked so it has no consequence that it is in a >>>>>> "throws" clause. >>>>>> Or do you want to not remove FunctionEvaluationException from the >>>>>> "throws" >>>>>> clause (because it is not a backward-compatible change)? >>>>>> >>>>> The "aroma" I was referring to is that MathUserException is not, >>>>> strictly speaking a suitable replacement for >>>>> FunctionEvaluationException. The intent as described in the javadoc >>>>> for MathUserException is that it allows exceptions in user-defined >>>>> functions to be propagated through [math] API layers (an excellent >>>>> idea, IMO). >>> >>> I somewhat agreed on this point, because it doesn't hurt, although, as said >>> earlier, I really doubt that we can set a standard. [Anyway IMO it's fine >>> that users create whatever exception they like.] >>> >>>>> The problem is that FunctionEvaluationException is >>>>> broader - it could apply to non-user-defined functions, as in the >>>>> interpolation code that Luc pointed out. >>> >>> I mentioned that the "interpolate" method creates an object that implements >>> the "UnivariateRealFunction" interface. >>> Unless I'm missing something, the "problem" you mention does not exist. The >>> actual problem was the very _existence_ of "FunctionEvaluationException": A >>> class that was almost never actually instantiated within CM (and in the >>> places where it was, it was the wrong thing to do). [And the fact that is >>> was a chacked exception made things worse: try/catch all the way up for >>> something that never happens! That's why I argued that it be removed.] >> >> I understand your point, > > I'm not so sure. Maybe I don't explain clearly. > >> but I disagree with it. We are back to a >> basic principle of API design that we need to settle. My view is that >> FunctionEvaluationException absolutely makes sense at the API boundary >> of UnivariateRealFunction#value. It is the right abstraction at that >> level - it says that an exception occurred evaluating a function. > > It is not because it doesn't convey any non-obvious information. > How is this > --- > try { > f.value(x); > } catch (FunctionEvaluationException e) { > console.warn(e); > } > --- > more informative than this > --- > try { > f.value(x); > } catch (MathRuntimeException e) { > console.warn(e); > } > --- > ? [I mean, you "try" to call a method that will _evaluate_ the function, so > that, when you "catch" something, it's, quite obviously, because the > evaluation failed.] In this trivial case, yes, it is obvious, but it is a contrived example. In real life applications, you will almost never have a catch just at the same level of the call and you will not have a single line that can fail. In real code, you have several layers of functions, even several layers of libraries, and in a try/catch clause the try contains lots of different lines, each of which being able to throw some exceptions. What we say is that classical use cases can be like this: double myOwnAlgorithmWhichCallsSolverAtSomeDeepLevel(double a, double b) throws FunctionEvaluationException { UnivariateRealFunction f = new UnivariateRealFunction() { double value(double x) { if (complexConditionIsNotMet(x)) { throw new FunctionEvaluationException(x); } return x + 1.0; } } return solver.solve(maxEval, f, a, b); } void topLevelFunction() { try { // numerous lines which use [math] and can fail a = ...; b = ...; // small part involving UnivariateRealFunction result = myOwnAlgorithmWhichCallsSolverAtSomeDeepLevel(a, b); // still numerous lines which use [math] and can also fail } catch (FunctionEvaluationException fee) { // here we known our complex condition was not met at some point // so we need to adjust a and b } catch (MathRuntimeException mre) { // some OTHER unknown error occurred, we don't know which one (yet) } } In this exemple, I put only two levels, in many application > > Following your rationale, one would have to create one exception for each > possible action (method). You have an API that would look like > > * "value" can raise an "EvaluationException" > * "interpolate" can raise an "InterpolationException" > * "solve" can raise a "SolveException" > * "integrate" can raise an "IntegrationException" > * "optimize" can raise an "OptimizationEception" > etc, etc. No. We need very few user exceptions. Up to now, we had only two of them: FunctionEvaluationException and DerivativeException. We considered it would be sufficient to have one MathUserException only. > > If you want to talk in terms of boundaries, I think that these abstractions > are on the other side of the CM boundary, i.e. they are useful to users of > CM within their own code. > On this issue, we have been in disagreement for a long time; I'm pretty sure > that this is because both Luc and you are heavy users of CM and you cannot > separate your role of developer of CM from the role of developer of > applications-that-use-CM. It may be so, but at least for myself I really try to think about API from user side and not from developer side. In fact, in our discussion I think I have always preferred a solution that was simple for users rather than simple for us, [math] developers. This very discussion is an example of that: I prefer we provide a MathUserException for user convenience, even if it implies having to document an exception we should NEVER thorw by ourselves (hence I really think Phil has the right solution when he suggest we change our current use of MathUserException by another exception in same hierarchy). > > In your application, you could have some code like > --- > import org.apache.commons.math.analysis.solvers.UnivariateRealSolver; > import org.apache.commons.math.analysis.solvers.BrentSolver; > import org.apache.commons.math.analysis.UnivariateRealSolver; > import org.apache.commons.math.exception.NoBracketException; > import org.apache.commons.math.exception.TooManyEvaluationsException; > import org.apache.commons.math.exception.MathRuntimeException; > import com.psteitz.nice.app.ApplicationFunction; > import com.psteitz.nice.app.exception.FunctionEvaluationException; > import com.psteitz.nice.app.exception.SolverException; > > UnivariateRealSolver solver = new BrentSolver(1e-4, 1e-6); > UnivariateRealFunction f = new ApplicationFunction(); > try { > solver.solve(20, f, 1, 3); > } catch(NoBracketException e) { > throw new SolverException("No bracketing"); > } catch (TooManyEvaluationsException e) { > throw new SolverException(new ConvergencException(e.getMax()); > } catch (FunctionEvaluationException e) { > throw new SolverException("Evaluation failed"); > } catch (MathRuntimeException e) { > throw new SolverException("Undocumented CM failure: " + e); > } catch (Exception e) { > throw new SolverException("CM bug: " + e); > } > --- > where > "NoBracketException", > "TooManyEvaluationsException", and > "MathRuntimeException" > are low-level exception classes defined in the low-level CM library > (describing the exact problem as encoutered by the CM code), and > "FunctionEvaluationException" and > "SolverException" > are appropriate abstractions for your application. > > The first two "catch" blocks are there because the CM library documents that > those problems can arise from calling "solve". > The third one is there because you (as application-developer) decided that > "AplicationFunction" can raise such an exception (and this > "FunctionEvaluationException" is not defined in CM; it is defined in > relationship with the "AplicationFunction" that can raise it). > The fourth is there as a security measure (for the application) in case > "solve" did not behave according to its Javadoc. [The security holds if CM > globally repects the policy that all exceptions are subclasses of > "MathRuntimeException".] > The last one protects the application from CM bugs. > > You have to let the application developer decide how low-level exceptions > translate to concepts useful to the application at hand, not the other way > around as it is impossible in all generality (because the low-level is > lacking the "context"). > > >> In >> some cases, for example in activating a solver, a caller will know >> that it is possible that an argument outside the domain of the >> function may be passed to the function (solving, for example, in a >> neighborhood that contains singularities). The caller may want to >> know simply that an error occurred evaluating the function. > > This is exactly my above example. where the "caller" you refer to is the > application code which knows perfectly what _specific_ exception can be > raised by the function it just asked CM to solve. Thus: the "catch" block > targets the "FunctionEvaluationException". > > It is wrong to imply, from this use-case, that the > "FunctionEvaluationException" which you used in your application is useful > to everyone. It is not. And certainly not to CM itself. But it is useful for some people at least (it is already used). We don't put in [math] only things that everybody needs and disregard the rest. > >> The >> nature of that error can be precised further by the exceptions >> hierarchy in one of three ways: a) narrowing the class (i.e., the >> actual exception may be an instance of a subclass of >> FunctionEvaluationException) b) unpacking a nested exception or c) >> examining state information or the exception message. All three of >> these options are available to us in designing the exceptions >> hierarchy and classes. I think it is naive and frankly bad design to >> aim to define only low-level runtime exceptions that report things >> like "NumberTooLarge." > > You are confusing two things: > (1) > It is bad design to let an exception propagate to layers where it is not > appropriate; instead, it should be caught and handled or wrapped (or > converted) into a more approriate abstraction before being rethrown. The point here is "where it is not appropriate". If we consider that at each level we should catch everything, just to wrap it and rethrow it even if we cannot put any added value, then we are back to what was used in Fortran code for 50 years: using error codes and managing them everywhere. Exceptions are used because you act on them only at two levels: the location where you throw them because it is there that you detected the problem, and the location at which you have something useful to do with it. It can be simply stopping everything at the top level, or it can be wrapping and rethrowing because you have added value there. There are many places where it is not appropriate to handle the exception, so we must let it propagate. > [Incidentally, this bad practise is encouraged by the CM localization > framework. Luc and you wanted to keep it because it makes a mid-level > application's code easier (i.e. skipping the catch/wrap/rethrow at the > mid-level and let the low-level exception show its localized message at the > top level). Easier, yes, but bad design nevertheless.] I don't get it. > (2) > Good design implies exceptions that are commensurate with the abstraction at > hand. So, there is nothing wrong with a low-level component code generating > low-level exceptions. It is not CM's job to take care of converting from > its level to the upper level. And if we do it, it will go wrong at some > point because we lack the context. Yes, but a user function is high level and should have high level exceptions even if it is called from a low level solver. Indeed, low level code can call high level code. > > You only think you can do it because you see it through the eyes of a CM > user. So are we too developer-oriented or too user oriented ? I prefer to be user-oriented. > CM reports failures, and users should be free to deal with them as > they wish: Either let it propagate as is or wrap it in whatever wrapper they > like. > >> The principle that I stated in my earlier post >> that exceptions should make sense at the in the context of each API >> boundary means that we need a substantive hierarchy that expresses the >> concepts appropriate at each level. > > That's wright. But what you propose does not achieve it because the > different levels are not within the same body of code. > Exception should either express a specific problem (that's true for the > low-level exception in CM) or be high-level abstractions needed inside CM > (as, for example, if some algorithm actually need to "catch" a family of > failure conditions). > >> FunctionEvaluationException, like >> ConvergenceException, is an example of a basic concept that we need to >> keep, IMO. > > As per all that precedes, we need not, and we should not. As per all that precedes, some of our users need them and we should provide it to them. > >>> >>> Back to the issue of the interpolators: When an interpolator implementation >>> encounters a problem, it must throw a specific exception that represents >>> that problem; it might be e.g. an "OutOfRangeException" (because the user is >>> trying to extrapolate) but it doesn't mean that "OutOfRangeException" must >>> be a subclass of a "FunctionEvaluationException" or even a subclass of an >>> "InterpolationException" (because, as a problem description, >>> "OutOfRangeException" is unrelated to those). These supposedly high-level >>> exception don't bring any new information to CM; they are empty shells. >>> >>>>> Making it the exception >>>>> thrown by UnivariateRealFunction#solve de facto limits the scope of >>>>> UnivariateRealFunction to user-defined functions. >>> >>> I don't get this. What is UnivariateRealFunction#solve ? >>> >> Sorry, I obviously meant "value." >> >>> Anyways, (guessing from the last part of the sentence) >>> "UnivariateRealFunction" is certainly not limited to user-defined functions. >>> Implementations (within CM and outside it) can throw *any* kind of unchecked >>> exception. [It's just that, as I had also pointed out, the documentation is >>> misleading for the unwary user (who might think that catching >>> "MathUserException" will prevent any blow-up).] >> >> The intent of this "misleading documentation" needs to be preserved, >> IMO. User-defined functions should throw MathUserException, [...] > > This is unenforceable. Just because it is not unenforceable does not mean we should not suggest it to users. Of course we cannot prevent them to throw any unchecked exception they want, this does not imply we should give up completely and not provide documented exceptions for the ones we expect to happen at some place. Putting a "throws FirstException, SecondException" in an API is a valuable information for users, even if in addition to these two expected exception there may be NullPointerException, ArrayIndexOutOfBoundException, WeirdUncheckedException1, WeirUncheckedException2. We cannot prevent the user to throw any of this but we do document what we expect. > This is actually the main point: Bloating CM with unused exceptions violates > the KISS principle (on the faint hope that all users will be happy with your > view of the use CM). > >> [...] a >> subclass of FunctionEvaluationExeption (new name) and the expectation >> should be that catching the top level class should in fact prevent >> things "blowing up" as a result of function activation. > > Why should a "MathUserException" be a kind of "FunctionEvaluationException", > and only that? I think there are very few cases of user code that can be provided to [math]. Functions are used in quadrature, root solvers and ODE solvers, and they deserve an exception. Events and StepHandler are used in ODE and they could deserve their own exceptions too. There are also visitors in the linear algebra API, but somehow I don't think a dedicated exception is useful for that, but it could. > >>> >>>>> The current 2_x >>>>> code cleverly replaces FunctionEvaluationException but still allows >>>>> user functions to throw it and user code to catch and handle it. The >>>>> problem is that it replaces it uniformly within [math] with >>>>> MathUserException. What might be better would be to replace >>>>> MathUserException with something like FunctionEvaluationException >>>>> (oops - that name is taken - need something else) and then have the >>>>> current MathUserException be a subclass, reserved for user functions. >>>>> Most internal signatures would then reference >>>>> FunctionEvaluationException2 (just kidding about the name, need >>>>> something else), but user functions would throw MathUserException. >>>> >>>> This seems a good idea to me, and I don't have ideas for a name of the >>>> higher level exception. >>> >>> No, no, no. >> >> Yes, yes, yes :) See comments above. > > No, no, no. Ditto. I hope we can finally come to a consensus. The basic problem is should [math] declare some users unchecked exceptions that it never uses by itself, so users are encouraged to use them when they want to throw them in their code and catch them in their code too without [math] getting on the way ? The current status (up to 2.1) is that this feature was present. The tally is Gilles is against it and wants to remove it, Phil and myself are in favor of it and want to preserve it. What do other [math] developers think ? best regards, Luc > > > Gilles > > --------------------------------------------------------------------- > 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