Affine Transform Inverse

Dear all,

I was checking out the implementation of AffineTransform2D. It turns out to check whether the matrix is invertible, the condition that is checked is:

final double det = a.det();
		/* similar to Jama, throw a RunTimeException for singular matrices. */
		if ( det == 0 )
			throw new RuntimeException( "Matrix is singular." );

which essentially implies that we would almost never see a case where matrix is singular.
Shouldn’t there be some sort of threshold here rather than checking against zero?

Moreover, the set(value, row, column) method right away checks for if the matrix is invertible with the new element, which is very odd.
So let’s say we’ve an original identity matrix, and I want to set to [[0, -1],[1, 0]]. So, I naturally start with set(0,0,0) and Bam! all of a sudden my code throws an exception saying matrix is not invertible, even though I’ve just only put one element inside! I know that the intent was to imitate Jama for some reason, but this behavior is unacceptable and dangerous!

2 Likes

Hi @Masoudas

I’ve run into that too. I agree that is annoying. Having that be a threshold there is probably a good idea.

A fairly easy workaround here is to use the set(double[]) method to set all the elements at once.

John

Thanks John for your comments

I think this problem needs attention in any case. And at least what I’m trying to do is to wrap this class in another class to take care of the bugs.

It’s a RuntimeException which is not dangerous, just ignore it, nobody is getting hurt. Don’t set elements one by one if what you plan to do is set them all. Use set(double…). If det is not 0, the matrix is invertible. This is not a bug.

1 Like

Thanks for the response @axtimwalde .

First, I think if the method is doing harm, either we should remove it, or take care of it. Otherwise we start to lose confidence in the library.

About determinant, there’s an entire chapter in numerical analysis books on matrix inversion, discussing how close we should be to zero (in terms of determinant or more precisely condition number) to be able to estimate inverse with good quality. So I’m gonna have to disagree with your statement. There should be a threshold here. As I stated, we never check for equality with doubles. This is wrong.

Last thing is that handling exceptions is a costly operation and must be avoided if possible. So I don’t agree with your statement saying just ignore the exception.

1 Like

Ok, let’s break it down in more detail.

  1. Yes, the inverse of almost singular matrices is shaky stuff and often not satisfying.
  2. Yes, handling exceptions is expensive.

but

  1. The method does not do harm, please define harm, I am not losing confidence in anything because of anonymous harm scares.
  2. Defining a threshold for where the inverse is considered shaky isn’t trivial because it degrades gradually, I am happy to accept an implementation of an algorithm from any non-anonymous chapter from any non-anonymous numerical analysis book, wrapped in meaningful API that allows programmers to understand the quality of their inverses better.
  3. If this evaluation is more expensive than calculating the determinant and throwing a RuntimeException it would have runtime implications that are more ‘harmful’ in practice than not having it (harmful as in slower).
  4. Any test of this kind will not ‘fix’ your use case where you temporarily make the matrix singular. This is an example for false use of correct API.
  5. Ignoring RuntimeExceptions is perfectly fine, that’s why they are not Exceptions. But again, you don’t really have this problem if you use the API appropriately.

Sorry for coming across so bully, I do not want to discourage you from investigating and improving this code, but I expect you do be diligent, precise, and logical in your report and to not throw around anonymous claims and references, and irrelevant use cases. It is clear that the code as it is prioritizes general case performance over handling difficult edge cases. Adding an arbitrary threshold will generate new edge cases where the result is wrong, a more comprehensive solution will most certainly result in increased runtime which is unacceptable for the general use case.

Can you please describe your actual use case where this behavior gets in your way? We already found that setting fields element wise is wrong when this is not what you intend to do eventually. It not only temporarily creates singular matrices but also triggers a matrix inversion every time you change a field. You want the matrix inverted only once, which can be achieved by setting all fields at once. None of the discussion above is relevant for this use case because you created truly singular matrices. Please add what other problems you have and we will find solutions for that.

2 Likes