You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
At the moment there are inconsistencies about how to handle and warn about large Operators being converted to matrices in some algorithms. For example LTIModel._poles warns you about this in certain cases, LTIModel.to_matrices does not.
I generally like the idea that there is a default for the size of matrix up until the conversion will be done. In pymor.models.iosys there is sparse_min_size that achieves something similar. I think it would be helpful to do this on a global level and bake this into to_matrix. Then all algorithms could be written more concisely without handling these cases and the behaviour of matrix conversion can be tuned to the computing platform in a central place.
The text was updated successfully, but these errors were encountered:
I would also be for having a more systematic way of dealing with this. I think it started with poles and hinf_norm (which moved to _poles and _linf_norm), and then was also added in MTReductor. I believe LTIModel.to_matrices is a bit different, because sparse matrices are not necessarily converted to dense matrices.
Another thing is that only warnings are raised. That makes it easy to shoot yourself in the foot, e.g., calling poles for a system of order 10,000 will call scipy.linalg.eig which is likely impossible to stop with Ctrl+C and you would need to somehow kill the Python process or wait for a very long time. (I think at some point the poles method had a force parameter, which would by default be False and raise an error for big orders, but then we removed it for some reason.)
For the first part, it sounds like a good idea to me to have a default option in to_matrix to specify when to warn about conversion from sparse to dense.
For the second part, one approach would be to raise errors instead of warnings. The user would then need to increase the size to avoid errors. That would make it annoying because of extra code, but would help avoid bad things from happening.
At the moment there are inconsistencies about how to handle and warn about large
Operators
being converted to matrices in some algorithms. For exampleLTIModel._poles
warns you about this in certain cases,LTIModel.to_matrices
does not.I generally like the idea that there is a
default
for the size of matrix up until the conversion will be done. Inpymor.models.iosys
there issparse_min_size
that achieves something similar. I think it would be helpful to do this on a global level and bake this intoto_matrix
. Then all algorithms could be written more concisely without handling these cases and the behaviour of matrix conversion can be tuned to the computing platform in a central place.The text was updated successfully, but these errors were encountered: