search

Macaulay2 / M2

The primary source code repository for Macaulay2, a system for computing in commutative algebra, algebraic geometry and related fields.
https://macaulay2.com
343 stars 230 forks source link

Missing documentation nodes in Macaulay2Doc #1668

Open mahrud opened 3 years ago

mahrud commented 3 years ago

After #1666 is merged, there will be 214 warnings left, and 1370 node are not subnodes, which would be good to improve.

mahrud commented 3 years ago

@DanGrayson could you document the 11 missing nodes at the top? I tried looking at a the source code for few of them, but I have no idea what most of them are intended to do.

DanGrayson commented 3 years ago

@DanGrayson could you document the 11 missing nodes at the top? I tried looking at a the source code for few of them, but I have no idea what most of them are intended to do.

Sure, but not soon.

mahrud commented 3 years ago

@DanGrayson when the items above are documented, we can switch to raising an error for items with missing documentation so that going forward pull requests are forced to document everything. Otherwise the list slowly grows.

DanGrayson commented 3 years ago

@DanGrayson when the items above are documented, we can switch to raising an error for items with missing documentation so that going forward pull requests are forced to document everything. Otherwise the list slowly grows.

Yes, indeed, we should do that.

DanGrayson commented 2 years ago

@mikestillman should document "columnRankProfile" -- I don't know anything about it.

DanGrayson commented 2 years ago

@mikestillman should document "rowRankProfile" -- I don't know anything about it.

DanGrayson commented 2 years ago

@mikestillman -- just tell me what those two functions do and I'll document them today.

DanGrayson commented 2 years ago

@mikestillman -- never mind, I found it online.

DanGrayson commented 2 years ago

@mikestillman -- shouldn't the row rank equal the column rank?

i30 : f = mutableMatrix {{1,2,3}, {3,4,5}, {4,6,8.} }

o30 = | 1 2 3 |
      | 3 4 5 |
      | 4 6 8 |

o30 : MutableMatrix

i31 : rowRankProfile f

o31 = {0, 1, 2}

o31 : List

i32 : columnRankProfile f

o32 = {0, 1}

o32 : List
DanGrayson commented 2 years ago

@antonleykin -- can you tell me what "toricIdealPartials" in Dmodules does? I'll document it for you today.

DanGrayson commented 2 years ago

@ggsmith @denham0 -- shall I rename "tolist" to "toList" for you in the package HyperplaneArrangements? And how shall we document its two methods?

DanGrayson commented 2 years ago

@mikestillman @luisgarciapuente -- if you tell me what globalMarkovStmts does in the package Markov, I can document it for you.

luisgarciapuente commented 2 years ago

The package Markov should no longer be used since GraphicalModels has superseded it for more than a decade. But in any case, globalMarkovStmts is now called globalMarkov which is documented

https://faculty.math.illinois.edu/Macaulay2/doc/Macaulay2/share/doc/Macaulay2/GraphicalModels/html/_global__Markov.html

mikestillman commented 2 years ago

Maybe we should move Markov to undistributedpackages? Or legacy packages? Or remove it entirely? I do have some old code that uses that package, so I might prefer one of the first 2 options, although I can try to get my old code working with the latest version (if all functionality appears in your "new" package. I checked at one point, but forget what I did, or noticed).

luisgarciapuente commented 2 years ago

In deed, even I have some old code that uses Markov that I have not taken the time to update to GraphicalModels. I also know that some people elsewhere still use it. So I also agree to move it to legacy packages but not to remove it.

But definitely at this point GraphicalModels has all the exported functionality of Markov and more. Also, through the years, we have fixed some bugs in GraphicalModels and some of those were legacy bugs from Markov.

Maybe we should add a note on Markov that users are strongly encouraged to use GraphicalModels instead?

DanGrayson commented 2 years ago

Well, if we're not going to remove the package "Markov", and we're going to keep distributing it, then we should at least make sure it works. (I'm working on making it an error for a package to have missing documentation nodes.) The symbol "globalMarkov" is in a different package, so even though it's documented, that doesn't help document globalMarkovStmts in this package.

We don't have a class of packages called "legacy packages". I think we don't need one, either. Just add a statement to the documentation of Markov that there is a modern replacement available.

antonleykin commented 2 years ago

@antonleykin -- can you tell me what "toricIdealPartials" in Dmodules does? I'll document it for you today.

This seems to create a toric ideal describing the image of the given monomial map.

i2 : examples gkz

o2 =
     A = matrix{{1,1,1},{0,1,2}}
     b = {3,4}
     I = gkz (A,b)
     describe ring I
     D = makeWA(QQ[x_1..x_3])
     gkz(A,b,D)
     toricIdealPartials(A,D)
     eulerOperators(A,b,D)

The exponents of the monomials in the monomial map here are the columns of A and the "partials" (differential variables) in D are used as variables in the polynomial ring that houses the output.

ggsmith commented 2 years ago

Since toList is a CompiledFunction rather than a method, it appeared (at least to us) that we could not overload it. The tolist method was created as a kludge. How does one overload a CompiledFunction?

We (Avi Steiner, @denham0 ,and @ggsmith) also anticipate making a pull request for a major revision of the HyperplaneArrangements package before the end of April. As a consequence, having you (@DanGrayson) edit the documentation for this particular method does not make much sense to me.

DanGrayson commented 2 years ago

@antonleykin -- thanks. I've added this:

document {
     Key => { toricIdealPartials, (toricIdealPartials, Matrix, PolynomialRing) },
     Headline => "image of a monomial map",
     Usage => "toricIdealPartials(A,D)",
     Inputs => {
      "A" => Matrix => { "a matrix over ZZ whose columns give the coefficients of a monomial map" },
      "D" => PolynomialRing => { "the corresponding Weyl algebra" }
      },
     Outputs => {
      Ideal => { "the toric ideal of the image the monomial map described by ", TT "A", " formed in the
           polynomial ring generated by the differential variables of ", TT "D", "." } 
      },
     EXAMPLE lines ///
      A = matrix{{1,1,1},{0,1,2}}
      D = makeWA(QQ[x_1..x_3])
      toricIdealPartials(A,D)
     ///
     }
DanGrayson commented 2 years ago

Since toList is a CompiledFunction rather than a method, it appeared (at least to us) that we could not overload it. The tolist method was created as a kludge. How does one overload a CompiledFunction?

Ah, I'll fix that by changing toList into a function with methods.

We (Avi Steiner, @denham0 ,and @ggsmith) also anticipate making a pull request for a major revision of the HyperplaneArrangements package before the end of April. As a consequence, having you (@DanGrayson) edit the documentation for this particular method does not make much sense to me.

Okay.

DanGrayson commented 2 years ago

Since toList is a CompiledFunction rather than a method, it appeared (at least to us) that we could not overload it. The tolist method was created as a kludge. How does one overload a CompiledFunction?

Ah, I'll fix that by changing toList into a function with methods.

That's been done in https://github.com/Macaulay2/M2/pull/2433 -- enjoy!

mikestillman commented 1 year ago

Ouch. Yes!

Sent from my iPhone

On Apr 12, 2022, at 4:24 PM, Daniel R. Grayson @.***> wrote:

 @mikestillman -- shouldn't the row rank equal the column rank?

i30 : f = mutableMatrix {{1,2,3}, {3,4,5}, {4,6,8.} }

o30 = | 1 2 3 | | 3 4 5 | | 4 6 8 |

o30 : MutableMatrix

i31 : rowRankProfile f

o31 = {0, 1, 2}

o31 : List

i32 : columnRankProfile f

o32 = {0, 1}

o32 : List — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

mahrud commented 1 month ago

@pzinn can you think of a way to either automatically document these or mark them as undocumented?

o48 = {0 => (**, Expression, TensorProduct)   }
      {1 => (**, TensorProduct, Expression)   }
      {2 => (**, TensorProduct, TensorProduct)}
      {3 => (*, Expression, Product)          }
      {4 => (*, Product, Expression)          }
      {5 => (*, Product, Product)             }
      {6 => (++, DirectSum, DirectSum)        }
      {7 => (++, DirectSum, Expression)       }
      {8 => (++, Expression, DirectSum)       }
      {9 => (+, Expression, Sum)              }
      {10 => (+, Sum, Expression)             }
      {11 => (+, Sum, Sum)                    }
      {12 => (-, ZeroExpression)              }
      {13 => (==, Equation, Equation)         }
      {14 => (==, Equation, Expression)       }
      {15 => (==, Expression, Equation)       }
      {16 => (expression, Expression)         }
      {17 => (hold, Expression)               }

(The list is just everything without documentation that inherits from an Expression).

Currently some methods like this are manually marked as undocumented, e.g: https://github.com/Macaulay2/M2/blob/ec9e9ac60ed4a8e791448942202f077a88a87e15/M2/Macaulay2/packages/Macaulay2Doc/doc12.m2#L284-L291 But maintaining this by hand is hard, and I want something straightforward like this: https://github.com/Macaulay2/M2/blob/ec9e9ac60ed4a8e791448942202f077a88a87e15/M2/Macaulay2/packages/Macaulay2Doc/doc12.m2#L293-L299

pzinn commented 1 month ago

yeah that shouldn't be too hard.