semigroups / Semigroups

The GAP package Semigroups
https://semigroups.github.io/Semigroups/
Other
23 stars 36 forks source link

Rename `Matrix` to something else #512

Closed james-d-mitchell closed 2 years ago

james-d-mitchell commented 6 years ago

To resolve the name clash introduced by GAP PR #2640, see also GAP Issue #2747, I propose that we rename Matrix to something else, perhaps MatrixOverSemiring?

This will, I think, resolve the problems we are currently having with GAP, and uncouple whether or not Semigroups works from further matrix object developments.

markuspf commented 6 years ago

I believe this will resolve the immediate clash with the GAP library.

I wonder whether we should even prevent MatrixObj to be a filter set for Semigroups matrix objects, as otherwise we'll get GAP methods selected for them and break things in unfun ways.

james-d-mitchell commented 6 years ago

True @markuspf. What were you thinking of?

wilfwilson commented 6 years ago

I'm not sure what is best @james-d-mitchell, and I don't feel strongly about this issue. Do you still intend the Semigroups matrices to eventually fit into the MatrixObj framework? (I've not had anything to do with either your matrix implementation in Semigroups, or the MatrixObj stuff in GAP, so I'm not up to date on everything).

james-d-mitchell commented 6 years ago

I'm not really sure what's best either @wilfwilson, on the one hand I'd like to fit into the MatrixObj framework, but on the other hand, at the moment is doesn't work properly (at least in its interactions with Semigroups) and I'm not sure I have the ability/will to influence its development.

This is part of the reason for the existence of the matrix stuff in Semigroups in the first place, I prefer writing code to unproductive hostile wrangling over minutiae, so I wrote the current code in Semigroups.

At present the thing that is causing the issue is the return value of inputs of the form Matrix(GF(2), [[Z(2)]]);. I don't exactly recall the issues, but it was a great deal of effort to get semigroups of matrices over finite fields to work in Semigroups (@markuspf wrote much of this code), and the solution we found involved wrapping lists of lists into objects created using Objectify and in the category IsMatrixOverSemiring and its subcategory IsMatrixOverFiniteField. It was not possible to get this to work with lists of lists, with the current setup in Semigroups. Perhaps we grossly misusing filters for method selection, but I don't believe we are, and that is, in my mind, a separate issue.

I don't currently see a way of resolving the different return values of Matrix(GF(2), [[Z(2)]]); without extensively refactoring some parts of Semigroups, I wouldn't know where to start such a refactoring given that the future of MatrixObj is uncertain (or at least I don't know what it is), or exactly what the point of it would be, given that what we have now actually works, and life is short.

Given that this is being forced on us, and taking all of the above into consideration, I think the current proposal is the lesser evil. But I'm open to suggestions and happy to be convinced otherwise too.

markuspf commented 6 years ago

I see the following things at work here:

I think currently the best way is to have code in Semigroups for matrices over semirings, and develop the correct abstractions. If the GAP library wants to integrate these, then it can happen in future, and if done correctly we can switch to MatrixObjs at some point (but also we can choose not to, as we have the code around). We can always watch what's happening in the GAP library if we care to do so.

wilfwilson commented 6 years ago

That sounds reasonable @markuspf. I don't have new suggestions for a name; MatrixOverSemiring should be fine I think, as @james-d-mitchell suggests.

To avoid IsMatrixObj being set to true for Semigroups package objects, does that just us require us to set IsMatrixObj to be false for our objects at creation?

markuspf commented 6 years ago

Since IsMatrixObj is a category, and objects don't change category durting their lifetime, we would need to create them outside that category.

james-d-mitchell commented 6 years ago

I agree with your comments too @markuspf I'll try to make a PR implementing this as soon as I can.

fingolfin commented 6 years ago

So, as I understand, you are opting to not use matrixobj at all. I am rather sad to hear that -- I was hoping that semigroups would be interested in helping us to make progress on matrixobj, and in particular, actively participate in the design of it, precisely to help avoid it being over-engineered, or not supporting things you need resp. doing things that are bad for you.

Now instead, it seems you will develop yet another matrix infrastructure in parallel to it, so we end up with three or more. :-(

It seems unfortunate to me that we seem to again and again have such a hard time coordinating efforts between the GAP team on the one hand, and the Semigroups team on the other, despite there even being an overlap in personal between the two, and several from each group being at the same university. What can we do to improve communication?

james-d-mitchell commented 6 years ago

What can we do to improve communication?

We could limit our interactions on github, and elsewhere, to constructive dialogue rather than sarcasm (on my behalf) or ranting (on yours @fingolfin) and other unproductive activities. The latter has cropped up time and time again in GAP development over the years, I find it reduces my motivation to engage in the development of the main GAP system to zero (or close to it), I am not interested in being on the receiving end of this type of thing.

With regard to the recent developments of MatrixObj, I will attempt to explain my negative reaction to this (in the spirit of improving communication). GAP PR #2640 broke (and continues to break) the Semigroups package, it seems to be the case that reverting PR #2640 is not desirable, at which point I became highly motivated to properly resolve the issue, and received no responses for several days, indicating to me that the authors of PR #2640 were not as motivated to fix the issue as me. I was left with the impression (correct or not) that the attitude of the authors of PR #2640 "we broke it, now you fix it" and "here's the pesky Semigroups package causing problems again".

I suppose that there was some discussion at some point about MatrixObj among the various contributors, but I was not one of them, and so I have no idea about what anyone thought nor why things developed the way they did. Some of the developments seemed to contradict the original spirit of MatrixObj and what is written in the source files.

As for future developments of MatrixObj, I'd be happy to contribute if I could understand:

  1. What are the aims of MatrixObj that motivated PR #2640 in the first place?
  2. What would technically be required to realise these aims in the particular use case of the Semigroups package.
  3. What a reasonable/expected timescale for all of this might be?

Obviously this requires discussion. I have, over my years of involvement in GAP, often found this type of discussion to be very time consuming, inconclusive, and sometimes hostile. Several of the main GAP developers are almost completely inflexible in their views, or have unexpressed but deep issues with the functioning of society which produce a viewpoint that I don't understand. In my opinion, the merging of PR #2640 into GAP master and the subsequent comments follow this pattern, even if the participants are not all the same.

I am not expecting to sit back and magically have whatever I am thinking about implemented by someone else, quite the opposite, I'd prefer to skip the unproductive parts of the discussion and get down to finding a solution. I could do this and submit a PR to GAP, but given how GAP development appears to me to operate, this has a high probability of being a waste of time.

I am not happy with the hacky solution of down ranking the methods in the library, and want to start as soon as possible working towards a better solution. Currently the only path I can see is to do more or less what @markuspf outlines: do what is required to fix what PR #2640 broke in Semigroups, if some parts of that prove useful for GAP, then they can become part of the main GAP library at some point in the future.

I am happy to be convinced that I am wrong on any of the points in this comment.

wilfwilson commented 6 years ago

Now instead, it seems you will develop yet another matrix infrastructure in parallel to it, so we end up with three or more. :-(

@fingolfin I want to point out that the code for matrices in the Semigroups package has been present for a number of years in a stable state, and so for us, continuing to use this code is the easy option. It's not like we are spending effort at the moment improving this infrastructure at the expense of spending effort on MatrixObj.

fingolfin commented 6 years ago

Hi @james-d-mitchell I missed your comment here initially (it happened during GAP Days, with insane activity levels across GitHub issues, so at some point I just archived all notifications I got), but Wilf and Markus drew my attention to this on Friday. I plan to reply to you to that in detail via email later today or tomorrow, but wanted to write now just to make clear that I am not ignoring you deliberately. Also, I am genuinely sorry that I offended you, which never was my intention (but of course intentions don't matter that much when communicating). I am unfortunately super busy right now working through piles on my desk (I just returned to work from 2 months of paternal leave), but I'll take time to carefully re-read everything that was written by me and others, and then I'll get back to you in private email.

Also, chances are good that I will be in St Andrews from October 22 till 25; if you happen to have time and were willing to sit down with me then and talk about it in person, I would most appreciate it. I was and am serious that I'd wish that communication between the semigroups and GAP team were better, and I honestly don't understand why it seems to be as bad as it is -- to be clear, that's not an accusation towards you or the overall semigroups team, clearly the GAP side is at least equally "at fault", if not more, so this is honest puzzlement and a genuine wish to improve things. Believe it or not, in total I really appreciate what you guys are doing, and you in particular, James.

Anyway, of course I also understand and accept if you are too fed up with the whole situation and don't want to see my face :-(.

Oh, and regarding MatrixObj: I thought I had offered here on GitHub that we could disable anything of that which causes a headache wrt to Semigroups in 4.10, to get time to figure out how to resolve it; I have not check whether I really did that or not, but well, here it is. I also made this offer to Wilf and Markus last week.

james-d-mitchell commented 6 years ago

@fingolfin I am not and was not offended, I'll be happy to see you in St Andrews, and to talk more about things then.

james-d-mitchell commented 2 years ago

Resolved in v5.0.0