cwatson / brainGraph

Graph theory analysis of brain MRI data
173 stars 51 forks source link

Weighted Graph Efficiency #17

Closed AndrewLawrence closed 4 years ago

AndrewLawrence commented 4 years ago

Hi Chris,

If I understand the package right, brainGraph universally stores strength-type weights (i.e. larger value = stronger connections), then converts these with xfm.weights when cost/distance type weights (larger value = weaker connection) are needed.

The efficiency function needs the latter cost/distance type weights, but the documentation doesn't specify this.

First proposal - to clarify the documentation:

weights: Numeric vector of edge weights; if NULL (the default), and if the graph has edge attribute weight, then that will be used. To avoid using weights, this should be NA. Note: weights must be transformed such that the strongest connections have the lowest weights (see xfm.weights)

Beyond the documentation, if a user makes a weighted brainGraph and just wants to know the efficiency it's relatively natural to just run efficiency(myGraph). This gives the wrong answer because strength weights are used rather than distance weights. The package intended way to get efficiency measures is set_brainGraph_attr which works fine because this function handles the conversion.

I wanted to see what you think about changing the efficiency function so expected results for a weighted brainGraph are obtained by default?

For example, have efficiency transform input weights by default, adding an argument with options from xfm.weights, something like: weight.xfm = c("1/w", "-log(w)", "1-w", "none"), with the "none" option allowing a graph with pre-calculated distance weights to be passed. Then, when efficiency is called within set_brainGraph_attr you could pass "none" to avoid repeated transformations for the block of graph measures that use distance weights.

What do you think? Happy to help with a PR if this would be of any use. Alternatively if it's just me that wants to do it this way I can just wrap the function :)

All the best,

Andrew

cwatson commented 4 years ago

Hi @AndrewLawrence , I wouldn't say it "universally" stores strength-type weights; it is entirely dependent on the user's input graphs. I have assumed throughout that brain networks will universally have strength-type weights (streamline/fiber counts or FA/MD/AD/RD/etc. in DTI, correlation/covariance in fMRI or structural covariance), but non-neuroscience users do use some functions as well.

You are correct that efficiency doesn't mention xfm.weights; this was a conscious choice as I had initially expected users to transform the weights on their own if calling the function in isolation (if necessary). This was partly out of desire to stick to the "Unix philosophy" of writing functions that do one thing and that are meant to be "chained" together.

However, you make a good suggestion and I think it makes sense and is straightforward enough to include in a future release. I am nearing completion of v3.0.0 and should be able to include this in that update. Chris

AndrewLawrence commented 4 years ago

Thanks, that makes sense. On reflection, 'stores' was a bit of an odd choice of word and suggesting universality was - as you point out - flat out wrong. I should have said something like 'the package expects the "weight" edge attribute of brainGraph objects to contain strength-type weights'.

Looking forward to v3.0.0.