Feature/immutable

[lessthanoptimal]

Thank you for your contribution to the EJML project.

Before submitting this PR, please make sure:

• [x] Your code is covered by tests
• [x] You ran ./gradlew spotlessJavaApply

[lessthanoptimal]

Previous requests for an ImmutableMatrix https://github.com/lessthanoptimal/ejml/issues/155 . Ok I can find only one, but I know this has come up several times previously.

[lessthanoptimal]

@JozsefKutas @ennerf @FlorentinD Does this look good? The PR adds an ImmutableMatrix which is an interface SimpleMatrix implements.

[lessthanoptimal]

Might rename it from ImmutableMatrix to ConstMatrix. Const is shorter...

[JozsefKutas]

Hi, I've just looked over the changes and I have a few thoughts.

At the moment, the ImmutableMatrix isn't actually immutable. A class is immutable if it comes with a gaurantee that instances can't be modified. There's a quick summary of what this implies at https://math.oxford.emory.edu/site/cs171/immutability/ (taken I think from Bloch's Effective Java).

With the current implementation, there's no guarantee that the underlying data of ImmutableMatrix will not be modified after the immutable matrix is created:

DMatrixRMaj matrix = CommonOps_DDRM.identity(2);
ImmutableMatrix immutable = SimpleMatrix.wrap(matrix);
matrix.set(1, 1, 0d);

It's closer to the unmodifiable collections returned by some methods in java.util. Unmodifiability is a weaker condition than immutablility, but is still useful (see https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/util/Collection.html#unmodifiable).

It's not strictly unmodifiable either, because it's possible to mutate elements by downcasting:

ImmutableMatrix immutable = SimpleMatrix.identity(2);
SimpleMatrix simple = (SimpleMatrix) immutable;
simple.set(1, 1, 0d);

If you go with this implementation then I'd change the name, just to make it clearer to the user what guarantees the class does and doesn't provide.

Alternative approaches would be:

• Have ImmutableMatrix extend SimpleBase, and throw an UnsupportedOperationException in public methods that mutate the underlying data. Make sure methods like wrap make copies. If any public mutator methods in SimpleBase are used internally in the implementation of non-mutator methods in SimpleBase (to prepare the returned matrix) then this will require workarounds. This is the approach taken by Guava (https://github.com/google/guava/wiki/ImmutableCollectionsExplained).

• Rename the current ImmutableMatrix interface, and use it as the parent for mutable matrices (SimpleMatrix) and immutable matrices. This is similar to the approach used in scala (https://docs.scala-lang.org/overviews/collections-2.13/overview.html). This approach is more complicated, but it avoids using runtime errors to enforce immutability.

Besides these issues, I don't understand the motiviation behind removing the type parameter from SimpleBase. It means errors like this aren't caught until runtime:

ImmutableMatrix matrix = SimpleMatrix.identity(2);
StatisticsMatrix scaled = matrix.scale(2d);
// Exception in thread "main" java.lang.ClassCastException: class org.ejml.simple.SimpleMatrix cannot be cast to class org.ejml.example.StatisticsMatrix ...

[FlorentinD]

In general I like the idea of having an immutable matrix. One use-case could be for input data which you want to make sure you dont modify by accident.

For the PR itself, I agree with Jozsef and I am also missing some tests which show the Exception is thrown.

Also, starting with the API over the SimpleMatrix makes sense, but I could also see this as useful for more concrete implementations as this is a feature more advanced users would use.

[lessthanoptimal]

Hi, I've just looked over the changes and I have a few thoughts.

At the moment, the ImmutableMatrix isn't actually immutable. A class is immutable if it comes with a gaurantee that instances can't be modified. There's a quick summary of what this implies at https://math.oxford.emory.edu/site/cs171/immutability/ (taken I think from Bloch's Effective Java).

Ok Immutable isn't the best description clearly. It's main design goal is to make it so that someone has to go out of their way to modify it. Kinda of like const in C++, which apparently is considered "a shallow immutable object" as you can strip it of the const property in various ways.

Trying to prevent someone from downcasting and modifying or any other "attack" I think is overkill. This is mainly to keep honest people honest and make it easy to see if someone is deliberately breaking the contract. What I don't like about Unmodifiable is that some of the checks are at runtime, which can result in a nasty surprise.

Maybe call it ConstMatrix?

• Rename the current ImmutableMatrix interface, and use it as the parent for mutable matrices (SimpleMatrix) and immutable matrices. This is similar to the approach used in scala (https://docs.scala-lang.org/overviews/collections-2.13/overview.html). This approach is more complicated, but it avoids using runtime errors to enforce immutability.

That's what currently done I believe.

Besides these issues, I don't understand the motiviation behind removing the type parameter from SimpleBase. It means errors like this aren't caught until runtime:

Just reverted that change. It was result of an earlier experiment that didn't go anywhere and I forgot to undo it.

Thanks for all the thoughtful comments!

@JozsefKutas

[lessthanoptimal]

See KalmanFilterSimple for an example of how this could be put into practice.

[lessthanoptimal]

Unless there's some additional feedback or suggestion for a better name, I'm going to merge this in soon. Planning on doing another release soon too.

[lessthanoptimal]

@JozsefKutas Do you know if sealed types are syntax sugar and generate bytecode compatible with Java 11? EJML compiles Java 17 but creates bytecode for Java 11. EJML will be stuck with Java 11 bytecode for the foreseeable future. Took forever to give up on Java 8.

[lessthanoptimal]

As a side note, I'm not really the target audience for ConstMatrix either. I mostly work with the low level API. I think it applies to people who attempt to restrict access as much as possible and feel the need to use final ever place possible. I also liked the Equations API but I think I'm the only user of that API after all these years. That is probably a good candidate to be removed in the future, at least if it wouldn't break a bunch of my own code.

[JozsefKutas]

Do you know if sealed types are syntax sugar and generate bytecode compatible with Java 11?

They're not just syntax sugar, so I'd guess they're not compatible.