wcorr guardrails?

[michael-petersen]

This enhancement concerns

EXP/expui/expMSSA.cc

Line 118 in 03caf5e

Eigen::MatrixXd expMSSA::wCorrAll()

With ssa = pyEXP.mssa.expMSSA(config, window, npc, flags) [fill in standard arguments for a MWE], one can run wcorr = ssa.wCorrAll() before reconstruction, resulting in an identity matrix with no warning to the user.

If the user does a standard reconstruction, e.g.

pcpair = [0,1]
ssa.reconstruct(pcpair)
wcorr = ssa.wCorrAll()

the wcorr matrix will be non-identity for only the first two entries, again with no warning.

I think we probably want to place some guards: perhaps have wCorrAll to truncate the wcorr matrix to only the non-identity region? Or throw a warning if run with no reconstruction? I'm happy to do a PR when we've decided what the tactic should be. And probably more documentation.

[The9Cat]

Huh. Good find.

For the first issue, there is a reconstructed boolean that should be checked before attempting to do a wCorr prior to reconstruction. So that's really a mistake (as in bug which is trivially fixed).

For the second issue, the idea was to preserve the ordering of PCs in the matrix. E.g. if pcpair=[0, 4] then the weighed correlation would only be computed for ev = 0, 4. But it would be very confusing if one then truncated the matrix to rank two.

[The9Cat]

Okay, let's say that in a different way: it would be confusing to me if the weighted correlation matrix were truncated to effective rank. But I'm willing to consider that...

[The9Cat]

Another thought: completely different. But I can be convinced that this is what one wants: a call to wCorrAll() does a full reconstruction for all PCs and computes the correlation matrix.

[michael-petersen]

Good point about truncating if e.g. pcpair=[0,4], that would be confusing. Another strategy would be to blank the array with nan values?

Agreed about checking reconstructed, I'll add that.

[michael-petersen]

Another thought: completely different. But I can be convinced that this is what one wants: a call to wCorrAll() does a full reconstruction for all PCs and computes the correlation matrix.

Thinking about the full reconstruction, we'd definitely want to add more functionality for specified reconstructions, but I kind of like this idea.

[michael-petersen]

@JASHunt, this is a issue we've discussed, maybe you have an opinion about what the best 'default' wcorr output is?

[The9Cat]

I think that when I wrote this code, I anticipated that the user would know that if they wanted a full weighted correlation matrix they would need to do a full reconstruction. So, now that I've fully digested this, I think that your second issue, is actually a misuse of the code but in a way that I did not anticipate.

[The9Cat]

The only reason that it's not a full reconstruction automatically is that reconstruction is computationally expensive. But I'm now thinking that this is what we really want: force full reconstruction on a wCorr() call.

One option would be to limit the PC count. E.g. have wCorr() and wCorrAll() take an optional argument that restricts the rank.

[The9Cat]

I could add logic to check whether the last reconstruction was a full reconstruction up to the desired rank. But I'm now fully convinced that this should be changed. It will be a some computational cost, but that's how it goes.

[JASHunt]

I agree the default should be the full reconstruction (despite the cost), with some option to change if the more advanced users want something more specific

[The9Cat]

At this point, the only think that makes sense to me for a matrix is the full reconstruction (or the reconstruction for the first n PCs) . Do you have something else in mind?

However, I can think of more advanced use cases that are not currently possible. For example: suppose I have two groups A=[0,1,2,3] and B=[4,5,6,7] . What is the wCorr(A, B)? We can't currently do that now. But I am tempted to write it at some point in the future.