Closed dPys closed 7 years ago
AFAIK there is no restriction. May be I am also wrong.
What happened when you comment out those lines or increase the tol level ? Have you tried ?
Yes, if I comment out those lines, everything works fine! Just wondering why those lines even need to be included to begin with? (I've either had to fork nilearn every time in the past or had to keep commenting them out manually upon each upgrade, so I was wondering whether that trouble may be something I could avoid in future versions if there is no strong reason to keep those lines in the public release...)
Or maybe those lines were included by mistake? Figured it was just stringent error handling that never raised issues since most nilearn users are only dealing with symmetric matrices from rsfMRI...
plot_connectome
is heavily used for plotting connectivity matrices in rsfMRI research. We haven't seen issues so far on this aspect.
It is more of a discussion now which should be agreed by developers whether to add a parameter to control for symmetric or asymmetric matrix.
Whether plot_connectome was designed exclusively for rsfMRI or with additional the hope of having the flexibility to deal with dMRI data down the road, it makes little sense to me to require graph symmetry as a rigid requirement either way. If the concern is that some rsfMRI matrices that are intended to be symmetric (but are mistakenly asymmetric due to user error) may 'slip under the radar' without this error handle, then in the very least I think that it should be replaced with a mere warning:
import warnings
if not np.allclose(adjacency_matrix, adjacency_matrix.T, rtol=1e-3):
warnings.warn("'adjacency_matrix' should be symmetric")
warning makes sense but let's see what others might think on this
Sounds good! :-)
Thx for the suggestion. What I don't figure out atm is what should be plotted in the case of an asymmetric matrix: given that there is no dis-symmetry in the display, only the symmetric part of the matrix will be plotted anyhow ?
...if I turn off the restriction I can produce an asymmetrical matrix. If it is on, then I cannot?
OK, I thought you were discussing about plot_connectome
that renders edges in MNI space; in that case, there is no straightforward way to render an asymmetric matrix.
If you are referring to nilearn.plotting.plot_matrix
, then I understand your concern, and +1 for allowing non-symmetric matrices.
Indeed issue is abt plot_connectome
. I don't know why square matrix came into picture.
Ah, sorry for the confusion! yes, there is no straightforward way to render an asymmetric matrix in nilearn, but I can still do that myself with matplotlib/numpy outside of nilearn, so that adj. matrix I posted was mostly just to demonstrate that these assym matrices can and do occur and in my opinion should be unrestricted across the board. Here is an example of what I can plot with plot_connectome if this is turned off:
That's really nice, but my question is: when you want to plot the link between i and j, which of the following values should one use: A[i, j], A[j, i],1/2 * ( A[i, j] + A[j,i]), max(A[i,j], A[k, i]) etc. ?
So I can better understand your question, are you asking what edges, under asymmetric conditions, should be plotted? If so, the answer is that that is an open question! Since this kind of asymmetrical graph is essentially a directed multigraph, I would say that any of those options would work. I like max(A[i,j], A[k, i]) or 1/2*(A[i, j] + A[j,i]). But for now, even a binarized edge could work such as the black edges that I've included in the plot above. Maybe eventually I could help you guys to write a function that plots two thin edges simultaneously and adjacent to one another in order to depict multigraph degree (i.e. for both A[i, j] and A[j, i])? Bottom line is that, there is still no reason that I can see to restrict this kind of plotting to uni-directional graphs...
I think we should keep the display of asymmetric edges aside for the moment.
Any of the decision rules that we've discussed so far amounts to symmetrizing the graph before display: plot_connectome(1/2 * (A+A.T))
, or plot_connectome(np.max(A,A.T))
etc.
In any case, it is unclear what should be displayed when an asymmetric matrix is given to the function, hence we should probably keep the code as is and let the user decide what he/she wants to display.
Sounds good for now. But can we still keep this issue open for future discussion?
Maybe the error message is not the right one, but I do believe that the plotting function that we have does not plot in an informative way directed graphs, hence it should refuse to do so. It's a question of managing users expectation.
If you know what you are doing, you can alway build a wrapped function that created symmetric graphs from assymetric ones to use the nilearn function.
That sounds very reasonable to me! I will go ahead and make a pull request for this sometime in the coming months, and perhaps then we can touch base about it again.
Cheers, derek;
Is there a conventional way of plotting asymmetric graphs?
I not, I think that the function should be prototyped outside of nilearn, and submitted for inclusion only once it has empirically shown to be useful.
I think that I'll go ahead and close this issue, to avoid clogging the tracker. We can come back at it when we understand this space better.
Is there any reason not to remove lines 1333-1334 from nilearn/plotting/displays.py?
The reason I ask is that I'm trying to use nilearn to plot my adj. matrices from dMRI-generated structural matrices which are fundamentally asymmetric (when using prob. tracking). I think nilearn's plotting would benefit greatly if it were more flexible in this regard.
Curious to hear your thoughts on this.
derek;