commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [math] last steps before releasing 2.2 ?
Date Sat, 29 Jan 2011 04:31:00 GMT
Hello.

> >> >>> 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.]

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.

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.

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.

> 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.
[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.]
(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.

You only think you can do it because you see it through the eyes of a CM
user. 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.

> >
> > 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.
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?

> >
> >> >  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.


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message