search

dolphin-acoustics-vip / artwarp

MATLAB program for automated categorisation of tonal animal sounds
https://github.com/dolphin-acoustics-vip/artwarp/wiki
GNU Lesser General Public License v3.0
7 stars 10 forks source link

Performance improvements to warp.m function #9

Closed RohanK22 closed 2 years ago

RohanK22 commented 3 years ago

Optimizing similarity matrix calculation with parfor loop and caching. Replacing max(array) function calls to max(array, array) calls to avoid type conversion from numbers to array.

olexandr-konovalov commented 3 years ago

Thanks @RohanK22, very nice!

I have noticed that this PR also includes commit from #8. Was this done intentionally (e.g. #9 will not work without #8) or accidentally?

Anyway, we don't need to change anything for now, let's review suggested changes first, and sort that out while merging them.

olexandr-konovalov commented 3 years ago

P.S. @RohanK22 See also my comment on writing commit messages https://github.com/vmjanik/artwarp/pull/8#pullrequestreview-809621518

RohanK22 commented 3 years ago

@alex-konovalov I think that's mistake on my end sorry. Both #8 and #9 are independent of each other. When creating a new branch I must have checked out a new branch from old-ctr-file-support branch. What do you suggest I do?

olexandr-konovalov commented 3 years ago

@RohanK22 no problem, you can use interactive rebase to delete the commit from the branch and then force-push it to your fork. Can find time to write detailed instructions right now, perhaps we can just try later together.

olexandr-konovalov commented 2 years ago

@RohanK22 happy with new version, thanks (and read of the performance improvements in the report).

But this now has two commits with same commit message. You need to rebase this (with `git rebase -i main) and have the 2nd one as a fixup commit (so it will be incorporated into the 1st one, with commit message from the 1st one).

RohanK22 commented 2 years ago

@alex-konovalov I am really sorry. I need some help with this. I tried to rebase and fixup but now it looks like I have two more commits with the same message. Could we fix it in the next meeting?

olexandr-konovalov commented 2 years ago

It's already too late to fix - we should practice rebasing and merging in some toy repository in our next meeting, to keep history clean and prevent redundant commits/merges.