Skip to content

Conversation

@agramfort
Copy link
Member

see discussion at #105

cc/ @Eric89GXL

as it's a fairly big change if as many people as possible could review if would be great.

I've simplified the read_raw_segment function

@larsoner
Copy link
Member

I think doing a hashing check on info['projs'] on each getitem (followed possibly by a recomputation of .projector) is probably worth the sligthly increased lag on that function. It seems like the cleanest way. It would also enable people to 1) load data with projectors, 2) compute additional raw/continuous projectors with the originals applied, and then still 3) write the data out /without/ the projectors applied. Being able to write out the data without the projectors applied but with the projectors in the file is an important step in our labe because it allows us to visualize what each projector is doing to verify that the calculations are working correctly.

@agramfort
Copy link
Member Author

yes I agree. I trivial way (not 100% robust) to hash a list of projs is for
example:

proj_hash = sum(p['data']['data'].hash() for p in projs)

if proj_hash changes then recompute raw._projector.

FYI

In [29]: %timeit sum(p['data']['data'].hash() for p in proj)
100000 loops, best of 3: 3.09 us per loop

@larsoner
Copy link
Member

Sounds good to me. We should probably also hash active/inactive, since it would be good if individual projectors could be turned on and off at will. I think it also makes sense to check the status of the .proj variable (True/False) before doing any of this---although this will make some redundancy with active/inactive (since setting all projectors inactive should have the same effect as setting .proj=False), I think it's worth it.

@agramfort
Copy link
Member Author

would you consider giving it a try on top of my branch? I am drowning with
other stuff this week...

@larsoner
Copy link
Member

I should be able to give it a shot. What's the best way to manage code here? Do I pull in a branch from your (agramfort) repo, #hack hack, then push the updated version to my repo?

@agramfort
Copy link
Member Author

I should be able to give it a shot. What's the best way to manage code
here? Do I pull in a branch from your (agramfort) repo, #hack hack, then
push the updated version to my repo?

yes

git fetch agramfort
git checkout -b fix_raw_proj agramfort/fix_raw_proj

hack hack hack

git commit -m "yeah it works"
git push origin fix_raw_proj

and send me an email to ask me to take a look :)

@larsoner
Copy link
Member

Alright, made some modifications that added hashing. I think it's okay to throw an error if the user has preload=True and they modify raw.info['projs'] themselves.

I've tried to figure out a way to save the raw data unprojected, and I'm worried that it will cause problems with the eventual use of apply_function, so for now I'm not going to worry about that.

I haven't gone through the test cases yet, but I wanted to see if you thought this was a reasonable direction first.

You can see the changes I put in at my repo:
larsoner@16c1cf0

@larsoner
Copy link
Member

larsoner commented Oct 1, 2012

I think this is getting there! See my comments in the other thread (sorry if I was supposed to switch to this one):
larsoner@16c1cf0#commitcomment-1927235

@agramfort
Copy link
Member Author

we start to converge indeed !

I although have to admit I prefer option 2 (I find it easier to explain and less magical):

raw = mne.fiff.Raw(args, preload=True, proj=True)  # keep original projectors
raw.filter(args)
raw_proj = mne.compute_proj_raw(args)
raw.add_projector(raw_proj)
proj_eog = mne.ssp.compute_eog_projectors(args)
raw.add_projector(proj_eog)  # we should also make sure this actually applies it when raw.proj=True
proj_ecg = mne.ssp.compute_ecg_projectors(args)

raw = mne.fiff.Raw(args, proj=False)
raw.filter(args)
raw.add_projector(raw_proj + proj_eog + proj_ecg)
raw.save()

@larsoner
Copy link
Member

larsoner commented Oct 2, 2012

Alright, that's fine by me. If you want to revert the changes, go for it, otherwise I can do it in half an hour or so hopefully. It should more or less just be moving the reload_whatever_i_called_it() code back into the initialization function...

@agramfort
Copy link
Member Author

I'm on it

@larsoner
Copy link
Member

larsoner commented Oct 2, 2012

That and simply removing from line 145 on in test_raw.py should do it.

@larsoner
Copy link
Member

larsoner commented Oct 2, 2012

from 145 on in the test_proj() function, that is

@agramfort
Copy link
Member Author

I got interrupted and finally pushed. However we have a failing test in test_ssp.py now.
see test_compute_proj_eog() function.

@agramfort
Copy link
Member Author

btw I had to merge as rebased failed...

@larsoner
Copy link
Member

larsoner commented Oct 3, 2012

The failure was due to trying to modify the projectors in raw() following a preload, which we no longer allow. Adding a cp.deepcopy() fixed it. I don't think users should expect the compute_exg_proj() functions to modify the raw() files directly in any case---they should just return the projs as requested---so I'm satisfied with this solution:

larsoner@2b7e465

Now I'm trying to figure out how to see and address the conflicts...

@larsoner
Copy link
Member

larsoner commented Oct 3, 2012

Yeah, not sure how to view them. My git skills could use some improvement :(

agramfort added a commit that referenced this pull request Oct 3, 2012
API : new Raw.proj that allows to apply SSP projections with accessing raw data
@agramfort agramfort merged commit 458311d into mne-tools:master Oct 3, 2012
@agramfort
Copy link
Member Author

merged !

@dengemann dengemann mentioned this pull request Jul 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants