Opened 14 years ago

Closed 14 years ago

#54 closed task (fixed)

corrections/review of freegb.lib

Reported by: Oleksandr Owned by: levandov
Priority: major Milestone: Release 3-1-0
Component: singular-libs Version:
Keywords: Cc: levandov, greuel, Oleksandr

Description

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.

  1. any theory/references/background/explanations of algorithms is/are missing!
  2. no descriptions of internal formats/global assumptions in header/help
  3. 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...

Attachments (4)

freegb.lib (51.4 KB) - added by Oleksandr 14 years ago.
my corrections/suggestions to freegb.lib
freegb.lib.diff3 (20.2 KB) - added by Oleksandr 14 years ago.
Inspecting changes 1: initial version, 2: my changes/sugestions 3: latest author's version
freegb.lib.merge (55.0 KB) - added by Oleksandr 14 years ago.
Merge of new library together with my comments/corrections
freegb.tst (5.4 KB) - added by Oleksandr 14 years ago.
My tests for freegb.lib

Download all attachments as: .zip

Change History (17)

Changed 14 years ago by Oleksandr

Attachment: freegb.lib added

my corrections/suggestions to freegb.lib

comment:1 Changed 14 years ago by 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.

  1. 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.

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

please specify what you mean precisely.

  1. 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

comment:2 Changed 14 years ago by Oleksandr

  1. 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.

comment:3 Changed 14 years ago by Oleksandr

Owner: changed from levandov to Oleksandr

comment:4 Changed 14 years ago by Oleksandr

Status: newassigned

comment:5 Changed 14 years ago by Oleksandr

Summary: corrections to freegb.libcorrections/review of freegb.lib

Changed 14 years ago by Oleksandr

Attachment: freegb.lib.diff3 added

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

Changed 14 years ago by Oleksandr

Attachment: freegb.lib.merge added

Merge of new library together with my comments/corrections

comment:6 Changed 14 years ago by Oleksandr

Cc: levandov added; Oleksandr greuel removed

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

comment:7 Changed 14 years ago by 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?

comment:8 Changed 14 years ago by Oleksandr

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

comment:9 Changed 14 years ago by Oleksandr

my notices (shortly):

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

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

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

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

  1. 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).

Changed 14 years ago by Oleksandr

Attachment: freegb.tst added

My tests for freegb.lib

comment:10 Changed 14 years ago by levandov

Cc: greuel Oleksandr added

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

comment:11 Changed 14 years ago by Oleksandr

Owner: changed from Oleksandr to levandov
Status: assignednew

comment:12 Changed 14 years ago by levandov

Status: newassigned

The current version of the library fits well into release.

comment:13 Changed 14 years ago by levandov

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.