corrections/review of freegb.lib

[Oleksandr]

there are still a lot of things left for authors to do with the library,
for instance:

1. convertions routines are sloppy: easy to deceive (no checks/error messages):
   

see corresponding examples.

2. any theory/references/background/explanations of algorithms is/are missing!
   
3. no descriptions of internal formats/global assumptions in header/help
   
4. lots(!) of global routines which are not in the header (some without help/example),
   

e.g. 'Serre' which is used in examples!!!!

more details are as usual in the attachment (with my corrections)
ps: i didn't look too deep into procs. other than in the header...

[Oleksandr]

my corrections/suggestions to freegb.lib

[dreyer]

From singular-team@…:

Hi,

this lib is going to be in the release.

Singular wrote:

#54: corrections to freegb.lib

---

Reporter: motsak | Owner: levandov Type: task | Status: new Priority: major | Milestone: Release 3-1-0

Component: singular-libs | Version: Keywords: | ---------------------------+------------------------------------------------

there are still a lot of things left for authors to do with the library,
for instance:

1. convertions routines are sloppy: easy to deceive (no checks/error messages):
   see corresponding examples.
   

ok, this will be fixed. Many of conversion things will be made static.

2. any theory/references/background/explanations of algorithms is/are missing!
   

this is not true. Theoretical description went into main documentation as Non-commutative subsystem -> Letterplace. The alignment and formatting still to be enhanced.

3. no descriptions of internal formats/global assumptions in header/help
   

please specify what you mean precisely.

4. lots(!) of global routines which are not in the header (some without help/example),
   e.g. 'Serre' which is used in examples!!!!
   

will be fixed.

more details are as usual in the attachment (with my corrections)
ps: i didn't look too deep into procs. other than in the header...

Thanks for analyzing the lib. With best regards, Viktor

[Oleksandr]

3. no descriptions of internal formats/global assumptions in header/help

please specify what you mean precisely.

please see the lib for this! i will write down it a bit later on.

[Oleksandr]

Inspecting changes 1: initial version, 2: my changes/sugestions 3: latest author's version

[Oleksandr]

Merge of new library together with my comments/corrections

[Oleksandr]

Please check the freegb.lib.merge for my comments/corrections.

[Oleksandr]

As far as i understand it, global inegers "uptodeg" and "lV" are actually part of letterplace-ring definition. Therefore please let me suggest the following improvement: to use ring attributes instead of global inegers. That is to use something similar to the following:

proc setLatterplaceAttributes(R, uptodeg, lV)
"USAGE: setLatterplaceAttributes(R, uptodeg, lV)
RETURN: ring with all set attributes
PURPOSE: sets attributes for a latterplace ring: 
@*      'isLetterplaceRing' = true, 'uptodeg' = uptodeg, 'lV' = lV
"
{
  // Set letterplace-specific attributes for the output ring!
  attrib(R, "uptodeg", uptodeg);
  attrib(R, "lV", lV);  
  attrib(R, "isLetterplaceRing", 1);  
  
  return (R); // Should return a ring with these attribues... 
}

Using this, the following code

  ring r = 0,(x(1),y(1),x(2),y(2),x(3),y(3),x(4),y(4)),dp;
  int uptodeg=4; int lV=2;
  export uptodeg; export lV;
  Liebr(x(1),y(1),2);
  kill uptodeg, lV;

turns into

  ring r = 0,(x(1),y(1),x(2),y(2),x(3),y(3),x(4),y(4)),dp;
  def R = setLatterplaceAttributes(r, 4, 2); setring R;
  Liebr(x(1),y(1),2);

(of course, advanced people may set these attributes directly - and thus eliminate the need for an additional ring in such a case, or simply call freegbRing here)

Moreover, you might consider to do it in "freegbRing" as well, that is, either to export those global integers or to return setLatterplaceAttributes(@R, d, nvars(save)) at the end. In this case the code like

 intmat A[3][3] =
    2, -1, 0,
    -1, 2, -3,
    0, -1, 2; // G^1_2 Cartan matrix
  ring r = 0,(f1,f2,f3),dp;
  int uptodeg = 5; int lV = 3;
  export uptodeg; export lV;
  def R = freegbRing(uptodeg);
  setring R;
  ideal I = Serre(A,1); I = simplify(I,1+2+8);
  I;
  kill uptodeg, lV;

will look like:

  intmat A[3][3] =
    2, -1, 0,
    -1, 2, -3,
    0, -1, 2; // G^1_2 Cartan matrix
  ring r = 0,(f1,f2,f3),dp;
  def R = freegbRing(5); setring R;
  ideal I = Serre(A,1); I = simplify(I,1+2+8);
  I;

It seems to me that this way code becomes clearer and implementational details - hidden from end-user, which is, as far as i know, a good programming practice :)

Alas, this nice feature doesn't work in Singular right now :((( (see ​http://www.singular.uni-kl.de:8002/trac/ticket/81) Should be easily fixable ...

Viktor, what do you think about such an improvement?

[Oleksandr]

The aforementioned problem was fixed by Hans right away!!! And thus this improvement works as well.

[Oleksandr]

my notices (shortly):

1. Due to "3.9.1 Procedures in a library, 5^{th rule" the following}

procedure names are not good: freegbRing (-> freeGbRing?),freegbasis (-> freeGbBasis?),Liebr (-> lieBracket?),Serre (-> serreRelations?)

2. As far as i can see, internal intermediate data formats (lists of

strings, result of freeGbasis etc.) are not clearly documented anywhere.

3. mod2str, lst2str, vct2str seem to have the same problem with zeros.

In addition i noticed minor bugs with the optional argument in mod2str, lst2str, vct2str, and that isVar does think that "2x" is a variable (that is, it has no checks for coefficients).

[Oleksandr]

My tests for freegb.lib

[levandov]

First of all, thanks for the proposal with attributes, it is now realized in this way

ad 1. Many procs have been renamed

ad 2. future work. result of freeGbasis is actually clear, since it is nothing else than the output of system("freegb"), prepared for better presentation

ad 3. used as it's described in the lib, these functions work well. Changes will come gradually

[levandov]

The current version of the library fits well into release.