Opened 14 years ago

Closed 14 years ago

#68 closed task (fixed)

corrections/review of dmodapp.lib

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

Description

Just to complete the list

Attachments (3)

dmodapp.lib.txt (45.2 KB) - added by Oleksandr 14 years ago.
my corrections to dmodapp.lib
dmodapp.lib.diff3 (36.3 KB) - added by Oleksandr 14 years ago.
Inspecting changes 1: initial version, 2: my changes/sugestions 3: latest author's version
dmodapp.tst (4.3 KB) - added by Oleksandr 14 years ago.
my tests for dmodapp.lib.

Download all attachments as: .zip

Change History (9)

Changed 14 years ago by Oleksandr

Attachment: dmodapp.lib.txt added

my corrections to dmodapp.lib

comment:1 Changed 14 years ago by Oleksandr

Type: defecttask

comment:2 Changed 14 years ago by Oleksandr

Owner: changed from somebody to Oleksandr

comment:3 Changed 14 years ago by Oleksandr

Status: newassigned

Changed 14 years ago by Oleksandr

Attachment: dmodapp.lib.diff3 added

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

comment:4 Changed 14 years ago by Oleksandr

My notices to the current version of the library (shortly):

Generally it seems that lots of checks are missing and there are undocumented implicit assumptions so that innocent users may easily cause your procedures to fail. Specifically:

. exporting something into the current ring may cause name conflicts (see my test for DLoc).

Please consider using other means for returning complicated sets of

results in DLoc: maybe a list or via object attributes?

. when introducing new ring variables, at the very least, please check for all possible parameter/variable name duplications (see my tests for SDLoc, annPoly)

NB: nowadays Singular warns about a ring variable/parameter name conflict in ring(list) and resolves it by renaming conflicting variable/parameter in generated ring... which may break some ring-variable-name-dependent functionality...

btw, it seems that the check in SDLoc is wrong since it only ensures that a half of ring variables are ok.

IMHO, disallowing some variable names is a fight againt a user. It is fearly easy to generate new unique variable names! For example, please, take a look at "chooseSafeVarName" from new "matrix.lib" (clearly there are also other possibilities) .

Minor issue: fl2poly doesn't check exponents.

Moreover, please let me also remind you that that some of the bugs in bfun.lib were due to this dmodapp.lib... please don't forget them when you will be fixing this lib: e.g. 'engine' changes options, slimgb doesn't work AS IS for qrings, parameters cause one a lot of trouble, there may be non-commutative base ring and something commutative living there, check for non-zero field characteristic...

Changed 14 years ago by Oleksandr

Attachment: dmodapp.tst added

my tests for dmodapp.lib.

comment:5 Changed 14 years ago by Oleksandr

One more notice: the "DISPLAY:" header is missing/its use is inconsistent for the following [rocedures: ann*, *Loc*.

comment:6 Changed 14 years ago by levandov

Resolution: fixed
Status: assignedclosed

A lot of things have been improved, in particular

  • checks of most assumptions (char 0, no qring; Weyl algebra or comm. algebra etc.)
  • description of libs enhanced and formatted to be not longer than 80 characters
  • DISPLAY header updated/created
  • engine has been reworked and does not change options (as well as many other procs)
  • there are new functions, partially moved to nctools.lib; new function bFactor will take care of the factorizations and formats which we need. However, to migrate to bFactor will take some time, since dozens of procs use this kind of stuff in different contexts
  • fl2poly fixed
  • engine has been uniquified et cetera

More specific: *Loc* procedures are very nontrivial and technical as well as the theory behind them. A user, who understands localization of D-modules, will be able to rename his object in case of error message. Thus, the only solution we can do in the spare time is the following (for SDLoc, which exports LD0 and BS to currRing)

if (defined(LD0)
defined(BS))

{

ERROR("Reserved names LD0 and/or BS are used. Please rename the objects.");

}

Again I stress, that user-friendliness is provided for common folk procedures. For heavy artillery the process of making things user friendly will take time.

Note: See TracTickets for help on using tickets.