Macaulay2 / Workshop-2020-Warwick

6 stars 9 forks source link

Confusing reference to gaussianRing in documentation #103

Closed olgakuznetsova closed 3 years ago

olgakuznetsova commented 3 years ago

Here it essentially implies that the input (Ring, List, List) works for gaussianRings as well https://github.com/Macaulay2/Workshop-2020-Warwick/blob/ffb2fb754e94c6083883b2c9a7c2703e6cb61b84/AlgebraicStatistics/MLE/GraphicalModels.m2#L2687-L2695

but the code explicitly prevents anything other then Markov rings https://github.com/Macaulay2/Workshop-2020-Warwick/blob/ffb2fb754e94c6083883b2c9a7c2703e6cb61b84/AlgebraicStatistics/MLE/GraphicalModels.m2#L956-L957

roserhp commented 3 years ago

There are two instances of conditionalIndependeceIdeal and the first one (which has only 1 list as input instead of 2), allows for gaussianRings:

https://github.com/Macaulay2/Workshop-2020-Warwick/blob/f3b61c4b310f92c6cc27b3b115236b17ad0744c5/AlgebraicStatistics/MLE/GraphicalModels.m2#L890

olgakuznetsova commented 3 years ago

I understand, but it says "if the second list is omitted, then it assumes that these are the integers from 1 to n... in the variables of the Gaussian Ring". This suggests that I could actually do (Ring, List, List) also for Gaussian Rings if for some reason I was interested in a subset of variables.

luisgarciapuente commented 3 years ago

If the ring is generated with a graph then the variables names (graph nodes) are recorded in the ring and they must match with the names used in the independence statements.

However, One can create a markovRing or a gaussianRing without a graph. For example R = markovRing (2,2,2,2) R=gaussianRing 5

In these cases, the names of the variables are not recorded. So this is an issues when trying to input an independence statement. The default is for the variables to be named 1, 2, 3, ... However, in the case of markovRing there is an additional feature. One can input a statement whose variables are named with arbitrary symbols, as long as there is a second argument which is the list of variable names.

luisgarciapuente commented 3 years ago

I agree that the documentation should be made more explicit to state that the extra feature only applies to markovRings, but in principle there is no other change that is needed.

The reason this feature is not available for gaussianRings is that the code to allow for this gets really complicated since we want to select certain rows and columns in the covariance matrix indexed by the variables. So when the variables are symbols, this is not that simple. So we made the decision to force the user to input independent statements where the variables are labeled by natural numbers.