Heavy-computation @property in Matrix class

[Johan S. R. Nielsen]

Hi

I came across the following in sage/matrix/matrix2.pyx:14367:

@property
def I(self):
r"""
Returns the inverse of the matrix, if it exists.
<snip>
"""
return ~self

Basically, I don't understand why this def should be a property, and it
seems to me to be problematic. Specifically:

1) This is a property that can throw an exception. Isn't that a problem?

2) This is a property that runs a heavy computation when called. Isn't
that a problem?

I came across this because it currently breaks tab-completion on
matrices in the Emacs sage-mode: perhaps due to a bug, IPython inspects
the properties when tab-completing, so when a property throws an
exception, no tab-completion is given. That means that invertible
matrices don't tab-complete in sage-mode. The terminal is OK, and I
don't know about other interfaces.

There are other @property defs in that class, like conjugate and
hermitian.

Best,
Johan

[Nils Bruin]

On Tuesday, April 26, 2016 at 1:53:41 AM UTC-7, Johan S. R. Nielsen wrote:

Basically, I don't understand why this def should be a property, and it
seems to me to be problematic. Specifically:

1) This is a property that can throw an exception. Isn't that a problem?

2) This is a property that runs a heavy computation when called. Isn't
that a problem?

I agree those things are a problem. I think I know why it was done:

Transposition of a matrix is often written as M^T . It's difficult to support that syntax, so using M.T seems like a nice approximation. Once you have that, doing the same for conjugate and conjugate-transpose is a small step. And now that we have all these properties for order-two matrix operations, it's only natural to introduce one for inverses too.

Given the ubiquity of tab-completion I agree that this is now probably a bad idea. I don't know if it's bad enough to break existing code, though. Perhaps deprecate their use? Even that has a cost.

[Johan S. R. Nielsen]

> I agree those things are a problem. I think I know why it was done:
>
> Transposition of a matrix is often written as M^T . It's difficult to
> support that syntax, so using M.T seems like a nice approximation. Once you
> have that, doing the same for conjugate and conjugate-transpose is a small
> step. And now that we have all these properties for order-two matrix
> operations, it's only natural to introduce one for inverses too.

That seems like a sensible explanation.

> Given the ubiquity of tab-completion I agree that this is now probably a bad
> idea. I don't know if it's bad enough to break existing code, though.
> Perhaps deprecate their use? Even that has a cost.

I think that such magic is bad, for all the properties (transpose,
conjugate, conj-transp, inverse). It is not helpful to newcomers to Sage
to see that, apparently, *some* methods on objects don't require
parentheses, while almost everything else does -- such exceptions might
seem convenient the first 10 seconds, but afterwards it becomes
confusing.

New-comers to Sage often meet matrices as one of the first things. The
impression that leaves is important. In particular, they should
definitely not experience exceptions to Python conventions that arise
nowhere else in Sage.

In the interest of moving Sage to where it should be, as fast as
possible, I would vote for deprecating the behaviour. As you say,
deprecation - and in the end, removal - has a cost, but the greater goal
of having a coherent Sage is more important, I feel.

For the record, I would be OK with supporting abbreviations of the
methods (i.e. Matrix.T() == Matrix.transpose()). I don't know how/if
that could be implemented simultaneously with deprecating "Matrix.T".

Best,
Johan

[Jori Mäntysalo]

On Wed, 27 Apr 2016, Johan S. R. Nielsen wrote:

> I think that such magic is bad, for all the properties (transpose,
> conjugate, conj-transp, inverse). It is not helpful to newcomers to Sage
> to see that, apparently, *some* methods on objects don't require
> parentheses, while almost everything else does -- such exceptions might
> seem convenient the first 10 seconds, but afterwards it becomes
> confusing.

+1 for this. No, at least +2.

> New-comers to Sage often meet matrices as one of the first things. The
> impression that leaves is important.

And one more +1 for this.

--
Jori Mäntysalo

[Dima Pasechnik]

well, deprecating ".T", etc, just because it breaks an emacs mode, looks like a huge overkill, especially from the vim camp :-)

Besides, ".T" is bog-standard all over the numerical world. Removing nice syntactic sugar should not be done so easily. 

I don't quite understand the problem however. Are you saying that emacs mode triggers an attempt to actually compute ".T", etc, when you do

tab completion?

 

Best,
Johan

[Johan S. R. Nielsen]

> well, deprecating ".T", etc, just because it breaks an emacs mode, looks like

> a huge overkill, especially from the vim camp :-\)

Ah, you misunderstand me. I'm not arguing against the sugar due to the
Emacs mode - I'm arguing against the sugar because it's Bad For
Consistency.

> Besides, ".T" is bog-standard all over the numerical world. Removing nice
> syntactic sugar should not be done so easily.

From that point of view, I would agree that there should be a *method*
"Matrix.T()". Leaving out the parentheses by making T a property is a
micro-optimisation which is bad for consistency reasons. The fact that
Matrix.I throws an exception in the general case is even worse.

There was a discussion on sage-devel a while back involving properties,
and multiple people posited that it was sick to make a property that
often throws exceptions...

> I don't quite understand the problem however. Are you saying that emacs mode
> triggers an attempt to actually compute ".T", etc, when you do
> tab completion?

The Emacs mode was simply how I stumbled across the issue, because it --
incidentally -- *does* currently break tab-completion. Exactly why this
happens is not clear to me, but apparently pressing m.<tab> in
ipython-mode will call all properties on m; if one of those throw an
exception, the method poll is cancelled and the lisp code determines
that m has no methods at all. m.I throws an exception if m is singular.

This is clearly a problem with ipython-mode! But as I said: I'm not
against the properties on Matrix due to that Emacs mode but because it's
inconsistent, and hence confusing, syntactic sugar.

Best,
Johan

[Simon King]

On 2016-04-27, Johan S R Nielsen <santa...@gmail.com> wrote:
> From that point of view, I would agree that there should be a *method*
> "Matrix.T()". Leaving out the parentheses by making T a property is a
> micro-optimisation which is bad for consistency reasons. The fact that
> Matrix.I throws an exception in the general case is even worse.

Would it be feasible to let properties return some kind of "Undefined"
or "NotImplemented" or "None", rather than throwing an exception?

But generally,
- I am -1 to deprecate properties in general (here I include lazy
attributes, which I find handy).
- I am +1 to introduce the policy that properties should never throw
exceptions (and thus to deprecate all properties that currently do
throw exceptions).

Best regards,
Simon

[Johan S. R. Nielsen]

> Would it be feasible to let properties return some kind of "Undefined"
> or "NotImplemented" or "None", rather than throwing an exception?

That would side-step the entire purpose of exceptions: catching errors
fast and where they occur. This is extremely bad for debugging, and yet
another source of inconsistency that we should definitely not be
teaching our prospective Sagesters.

And the benefit of this sidestepping is to omit an empty pair of
parentheses?

> - I am -1 to deprecate properties in general (here I include lazy
> attributes, which I find handy).

Can you give me an example of a handy lazy attribute, to make it
concrete for me?

Essentially, I find the use of properties to go directly against a
general pattern in Sage (and OOP): that private fields are only
accessible through methods. For instance, there is no Matrix.nrows --
there is Matrix.nrows(). Why is this not a property? Consistency! In
sage.coding, we also recently refactored everything so that length,
dimension, etc. of a code is private fields, publicly accessible through
method calls. Everywhere I look in Sage, we have methods, not
properties. Why should these four examples in Matrix be so special?

Best,
Johan

[VulK]

One advantage of properties over methods is that they autocomplete
S.

* Johan S. R. Nielsen <santa...@gmail.com> [2016-04-27 13:22:57]:

> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

[Johan S. R. Nielsen]

> One advantage of properties over methods is that they autocomplete

I don't understand? Tab-completion covers both methods and properties in
all IDEs I'm aware of.

Best,
Johan

[VulK]

This example is dumb because it does not give any real benefit but it will
do. On the command line you could run something like

sage: B = matrix()
sage: B.T.roo<Tab>

and get

sage: B.T.rook_vector

but

sage: sage: B.transpose().roo<tab>

will complete to useless stuff.

Now as I said this does not make a lot of sense with matrices. On the other
hand suppose you have a mathematical structure A endowed with building blocks
S and a pointer to the current block. Suppose also that each building block
has some method defined. Then properties allow to autocomplete things like

sage: A.current_S.some_method()

The same example does not work if A.current_S is a method itself and not a property.

By this I do not imply that properties should be preserved in sage. I am only
answering your question on which benefit is there in omitting the
parenthesis.

Best
S.



* Johan S. R. Nielsen <santa...@gmail.com> [2016-04-27 13:46:08]:

[Johan S. R. Nielsen]

VulK writes:
> sage: B = matrix()
> sage: B.T.roo<Tab>
>
> and get
>
> sage: B.T.rook_vector

I see, didn't know that.

Of course, the reason that IDEs follow this is the underlying convention
that

a) Properties are blazingly fast to compute.
b) Properties don't throw exceptions.

Neither of which are satisfied for Matrix.I. If B is a 1000x1000
matrix over ZZ(x), then B.I.roo<Tab> is going to hang your IDE for a
while :-)

Best,
Johan

[VulK]

An interested question: my example from before was not extremely far fetched:
I am working precisely on something of that sort. A is a Cluster algebra, S
are its seeds and current_seed is a pointer to the current seed. Would it be ok
for future sage integration if I were to implement A.current_seed as a property or
should I stick to methods?
Thanks
S.


* Johan S. R. Nielsen <santa...@gmail.com> [2016-04-27 14:24:33]:

[Bruno Grenet]

+1 to everything Johan is writing. I think that the example B.I.roo<tab>
that hangs forever (or almost) is particularly striking.

As an aside, I agree that deprecation has a cost, and this observation
shows up in several discussions these days on sage-devel. For general
changes, like deprecate the use of properties for the end user, or
change the way packages can be installed, etc., we may take the
opportunity of the future switch from Python 2 to Python 3 to implement
them. Thus we could then advertise that from version 10.0 (say), the
interface of Sage changes (in a backward incompatible manner, due in
particular to Py2>Py3). If this comes together with a clear document,
easily found on sagemath.org, etc. that explains all the changes, it
would minimize the pain caused to the users.

Bruno

[Marc Mezzarobba]

Johan S. R. Nielsen wrote:

> 1) This is a property that can throw an exception. Isn't that a
> problem?
>
> 2) This is a property that runs a heavy computation when called. Isn't
> that a problem?

My two cents: both are problems, and matrix.I is problematic for this
reason. However, things like matrix.T that don't have these problems(?)
are fine with me (though I think ideally matrix.T should return a
*view* on the transpose). A lazy variant of matrix.I that wouldn't
compute the inverse right away would be okay too.

--
Marc

[Simon King]

Hi Johan,

On 2016-04-27, Johan S R Nielsen <santa...@gmail.com> wrote:

>> - I am -1 to deprecate properties in general (here I include lazy
>> attributes, which I find handy).
>
> Can you give me an example of a handy lazy attribute, to make it
> concrete for me?

Basically you spare the calling overhead, which in some applications has
made a difference.

OTOH, in all these cases that I can recall, one could argue that the
properties in question were *not* really part of the API, and were
still accessible through methods.

So, let me refrase:
- I am -1 to deprecate properties in general --- but properties shouldn't
be considered part of the API. And I guess that means throwing exceptions
and running long computations is fine, because tab-completion should only
show what's part of the API, isn't it?

Best regards,
Simon

[Nicolas M. Thiery]

On Tue, Apr 26, 2016 at 10:53:37AM +0200, Johan S. R. Nielsen wrote:
> I came across this because it currently breaks tab-completion on
> matrices in the Emacs sage-mode: perhaps due to a bug, IPython inspects
> the properties when tab-completing, so when a property throws an
> exception, no tab-completion is given. That means that invertible
> matrices don't tab-complete in sage-mode. The terminal is OK, and I
> don't know about other interfaces.

Just for the record, this also breaks introspection in the
IPython terminal and notebooks:

sage: sage: m = matrix([[1,1],[2,2]])
sage: sage: m.*rank*?
...
ZeroDivisionError

which has been annoying me for a while. Thanks for bringing this up!

Cheers,
Nicolas
--
Nicolas M. Thiéry "Isil" <nth...@users.sf.net>
http://Nicolas.Thiery.name/

[Nicolas M. Thiery]

On Wed, Apr 27, 2016 at 02:11:46PM +0200, VulK wrote:
> This example is dumb because it does not give any real benefit but it will
> do. On the command line you could run something like
>
> sage: B = matrix()
> sage: B.T.roo<Tab>
>
> and get
>
> sage: B.T.rook_vector
>
> but
>
> sage: sage: B.transpose().roo<tab>
>
> will complete to useless stuff.

I remember William mentioning a while ago on sage-devel a discussion
he had with Fernando about this. Fernando was saying that in IPython
they had deliberately not implemented tab completion involving results
of method calls (as above), because it would break too often (as we
are witnessing with our properties hanging or raising
exceptions).

This sounds like a sane default indeed. Yet this is very annoying
in Sage given our heavy usage of method calls.

So, I am wondering whether we could discuss this again with the
IPython/Jupyter team, so that users could explicitly request tab
completion on results of method calls. This could be through some
global option, a specific gesture (such as hitting tab several times),
or an interactive question ``do you really want to tab complete on the
result of a method call?''.

Thinking twice about it, I guess I'd like the following: hitting tab
once raises the question ``do you really want to tab complete on the
result of a method call? hit <tab> again if yes. this may break horribly.``

I changed the subject of the thread, as we are deviating topic.

[Volker Braun]

IPython (and hence Sage) can do tab completion of function call results already, its just off by default (thankfully!)

sage: %config IPCompleter.greedy=True

sage: sin(x).[TAB]

Display all 211 possibilities? (y or n)

[...]

[William Stein]

For the record, a third option is to attempt to tab complete for 3
seconds (say), and interrupt if the result can't be computed that
quickly.

But I think Volker's answer is sufficient for now.

William

> --
> You received this message because you are subscribed to the Google Groups
> "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

--
William (http://wstein.org)

[Martin R]

I am using emacs/sage-mode only, so my experience is biased.  What I would like though is that tab completion automatically adds at least the open parenthesis, or possibly both, whenever appropriate.

Would this make sense and would this be possible?

Perhaps one could do even more - I don't know whether autocompletion knows whether the method takes mandatory arguments or not, but if so, its behaviour could even depend on this...

Martin