#5 Now matlab scripts run correctly with modern matlab

sedurCode commented 3 years ago

This PR updates the three matlab scripts to work with modern matlab. The gitignore is also set up to ignore the common temporary files of matlab and python. It is assumed that however you install ffmpeg by Brew, apt or downloaded binary, that ffmpeg will end up on the system path, therefore we are no longer 'locked' to opt/lib/usr... Scripts should now perform the correct system calls on unix, mac and windows.

google-cla[bot] commented 3 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

sedurCode commented 3 years ago

@googlebot I signed it!

dicklyon commented 3 years ago

Yes, it's OK, either as is or by rectifying; nap is real, but not quite nonnegative as expected (due to the highpass in the IHC, I suppose).

image(63 * max(0, smooth_nap)' .^ 0.5)) % Rectify since nap can go slightly negative.

On Tue, Mar 2, 2021 at 4:30 AM Simon Durbridge notifications@github.com wrote:

@sedurCode commented on this pull request.

In matlab/CARFAC_binaural.m https://github.com/google/carfac/pull/6#discussion_r585523411:

[CF_struct, nap_decim, nap] = CARFAC_Run(CF_struct, test_signal, agc_plot_fig_num);

% Display results for 2 ears: for ear = 1:n_ears smooth_nap = nap_decim(:, :, ear); figure(ear + n_ears) % Makes figures 3 and 4

image(63 * ((smooth_nap)' .^ 0.5))

image(63 * abs((smooth_nap)' .^ 0.5))

I need to dig a little deeper, but Im guessing for the meantime its probably ok to rectify negative nap values? We aren't modelling actual neuron firing action potentials.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/google/carfac/pull/6#discussion_r585523411, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGW7DDSKY6JCOTAA3NN6EDTBTK6NANCNFSM4XOSHLQA .

sedurCode commented 3 years ago

% Rectify since nap can go slightly negative.

Change added. If there are no more change requests let me know and I can shrink it down to one commit.

dicklyon commented 3 years ago

All good by me. Thanks.

sedurCode commented 3 years ago

Squashed to one commit.

sedurCode commented 1 year ago

Hi @dicklyon I've just had a look at this stale PR from a couple years ago; Is this still valid, and are the changed still valid, or shall I close this PR?

dicklyon commented 1 year ago

Simon, I'll try to get one of my software engineers to figure this out. Thanks. Dick

On Thu, May 18, 2023 at 7:16 AM Simon Durbridge @.***> wrote:

Hi @dicklyon https://github.com/dicklyon I've just had a look at this stale PR from a couple years ago; Is this still valid, and are the changed still valid, or shall I close this PR?

— Reply to this email directly, view it on GitHub https://github.com/google/carfac/pull/6#issuecomment-1553128446, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGW7DE6YHNW5PE2I4YCAX3XGYVKFANCNFSM4XOSHLQA . You are receiving this because you were mentioned.Message ID: @.***>

dicklyon commented 1 year ago

I don't really know what facilities matlab has for file path handling, but sounds like a good idea. Dick

On Fri, May 19, 2023 at 2:49 AM Honglin Yu @.***> wrote:

@.**** commented on this pull request.

In matlab/CARFAC_SAI_hacking.m https://github.com/google/carfac/pull/6#discussion_r1198768530:

@@ -64,9 +71,5 @@ %% png_name_pattern = 'frames/frame%05d.png'; MakeMovieFromPngsAndWav(round(frame_rate), png_name_pattern, ...

wav_fn, ['CARFAC_SAImovie', wav_fn(1:end-4), '.mpg'])

-%% -system('rm -r frames');

wav_fn, [wav_fn(1:end-4), '.mpg'])

Can we use something like fileparts to bring back the CARFAC_SAImovie prefix? https://www.mathworks.com/help/matlab/ref/fileparts.html

— Reply to this email directly, view it on GitHub https://github.com/google/carfac/pull/6#pullrequestreview-1434122523, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGW7DBRBW7W5K7BZIOD3MDXG46Y7ANCNFSM4XOSHLQA . You are receiving this because you were mentioned.Message ID: @.***>

sedurCode commented 1 year ago

Now that I have more time, I'll try to address your comments in a following commit

google / carfac