Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code to select the aod motion computation method based on file versio… #16

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

saumilpatel
Copy link
Member

…n. Supports file version in which motion data are 3D

startup.m Outdated
setPath ;
setenv('DJ_HOST', 'at-database.ad.bcm.edu');
setenv('DJ_USER','spatel');
setenv('DJ_PASS','spatel');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argh! checked in password. Please change your password and exclude your startup.m from the path or set the environment variables in a separate script such as your ~/.profile (on Mac OS)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argh!, removed the startup from the location where it was stored, moved the credentials to matlab's default startup on the machine, also changed password

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is probably worth doing a squash of these two commits together and then doing a force push to your repository to overwrite them, so when the code is merged that never shows up in the history

analyzenthframe = 1 ; % skip frames if > 1
numframestoaverageforref = 10 ; % frames to average to determine the reference frame against which motion is computed
usemulticore = true ;
postrefstartframetime = 0 ; % N secs to skip before analyzing motion signals, in many cases, right after starting the point scan, there is a slight drift
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for better readability, use camelCase or underscore_separated compound names.

count = 0 ;
for ii=numframestoaverageforref+analyzenthframe:analyzenthframe:size(dat2,1)
count = count + 1 ;
for jj=ii-analyzenthframe+1:ii
Copy link
Member

@dimitri-yatsenko dimitri-yatsenko Feb 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can replace this loop with array operations:

adat(count,:) = adat(count, :) + mean(dat2(ii-analyzenthframe+1:ii,:))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced


% take into account frames to skip, i.e. sampling rate of motion,
% downsample with averaging
adat = zeros(size(dat2)) ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace these loops with matlab's implementation of downsampling.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did not change, cant find downsampling code that will average samples, dont want to deal with filters

needreference=false ; % no need for a reference, start computing positions relative to the reference of spheres in subsequent motion frames
if (usemulticore)
parfor ii=1:count
[shifts(ii+numframestoaverageforref,:)] = computeshifts(adat(ii,:)',coordinates,gridsize,calibparams,refparams,needreference) ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need brackets [ ... ] around shifts(...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

adat = zeros(size(dat2)) ;
if (analyzenthframe>1)
count = 0 ;
for ii=numframestoaverageforref+analyzenthframe:analyzenthframe:size(dat2,1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop may ignore the tail of the dat2 when analyzenthframe>1. Is that intended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont follow, but my best guess is what you mean is that samples at the end may not be processed, ?

numsamples = br1.sz ;
duration = numsamples(:,1)/br1.Fs ; % sec

%numframes = size(dat,1) ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commented code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

% requires z:\libraries\matlab in path
run(getLocalPath('/lab/libraries/hdf5matlab/setPath.m')) ;

analyzenthframe = 1 ; % skip frames if > 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename analyzenthframe to something like downsampleFactor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed

unknown and others added 3 commits February 21, 2017 10:25
2. replaced downsampling inner loop with vector operation
3. removed commented code
4. matlab style changes
Nothing done here affects functionality
coordinates = br.motionCoordinates;

fn = char(fetchn(acq.AodScan & key, 'aod_scan_filename')) ; % get the filename from db
oldversion = aod.checkFileversion(fn) ; % check if motion data is in old format, i.e. 2 planes or in new format, i.e. a volume
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a typo, or more likely the checkFileVersion should have been checked into the aod directory instead of the acq directory.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops... yes a typo, should be acq.checkFileVersion

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's probably best to keep with the aod schema since it doesn't apply to anything else in acq. Also, if we can refactor it into the AodMotionReader class so that this isn't exported in general that would be better (see comment below)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree... will move it


%% test function computeshifts
% requires z:\libraries\matlab in path
run(getLocalPath('/lab/libraries/hdf5matlab/setPath.m')) ;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be here. the user should have set up the path correctly before hand. this would likely have unintended side effects.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, will remove

function oldversion=checkFileVersion(fn)

oldversion = true ;
try
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is failing for me when analyzing

M/Mouse/2016-03-15_13-56-05/AODAcq/2016-03-15_14-38-45_%d.h5

specifically the h5readatt fails so we fall through to the catch statement. i believe this experiment was recorded with the new format, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh... works fine for the file you mention on at-ssp, Matlab R2016b. Is h5readatt working for other files ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check it from a mac? i'm using this from my local machine (R2016a but that command was introduced in 2011a)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, h5readatt does not work on my Mac 2016a... let me check why

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it works if the file name is /Volumes/M....
Is this what you meant in the comment getFile... To translate the file name for the platform used ?

useMultiCore = true ;
postRefStartFrameTime = 0 ; % N secs to skip before analyzing motion signals, in many cases, right after starting the point scan, there is a slight drift

br = aodReader(fn, 'Motion') ;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here I think this should use getFile(fn, 'Motion') to convert the path from /raw/blah to the real location

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this I am not following...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up replacing this with

fn2 = findFile(RawPathMap, fn)
br = aodReader(fn2, 'Motion');
br1 = AodMotionCorrectionReader(fn2) ; % if motion was corrected by using aod offsets, this is where they would be read

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good

calibparams = [0 1; 0 1; 0 1] ; % dont apply any calibration
shifts = zeros(size(dat2,1),3); % store motion vectors here
dat2(1,:) = mean(dat2(1:numFramesToAverageForRef,:)) ; % average frames for computing reference
[shifts(1,:), refparams] = computeshifts(dat2(1,:)',coordinates,gridSize,calibparams,refparams,needReference) ; % this position of the fitted sphere is treated as the reference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function is missing. above it references it being in lab\libraries\matlab but i don't see it there. also, please let's not spread these dependencies across multiple repositories.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in z:\libraries\TwoPhoton\AOD_Control\MotionCorrection. Note that we are using the same motion detection code that is used from the Labview program during real-time acquisition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like duplicating code in two places, but at the same time in this case I think it would be better than spanning dependencies between two pretty separate projects. It also avoids unintended side effects if one of the formats changes the other would break.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, would make the sessions folder self contained but we will have to remember to keep the two folders synced.

postRefStartFrameTime = 0 ; % N secs to skip before analyzing motion signals, in many cases, right after starting the point scan, there is a slight drift

br = aodReader(fn, 'Motion') ;
br1 = AodMotionCorrectionReader(fn) ; % if motion was corrected by using aod offsets, this is where they would be read
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need a separate class here just to access the sampling rate? can we not expand the original AodMotionReader to handle both formats correctly? I only see br1 used below to access the number of samples and sampling rate, which AodMotionReader should just be taught to do correctly for this new format.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not but we are just reusing code that we have.

@@ -0,0 +1,86 @@

function [x,y,z,t] = trackMotion3D(fn, analysischan)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we refactor this a bit so move all the code that deals with the raw file access into ScanMotion (like it is for the original code) and then make this method just take the data set and process it? that would keep the two consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to run motion detection during realtime acq as well as during offline analysis. Because realtime implementation came first, the code is residing in folder structure that we have for realtime stuff. We can rearrange if it is absolutely necessary, for sure we dont want to duplicate the same code in different folders

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.

4 participants