Skip to content

Parallel nested CV SVM training using a single OMP threadpool#254

Merged
MatthewThe merged 26 commits intopercolator:masterfrom
johnhalloran321:master
Apr 23, 2020
Merged

Parallel nested CV SVM training using a single OMP threadpool#254
MatthewThe merged 26 commits intopercolator:masterfrom
johnhalloran321:master

Conversation

@johnhalloran321
Copy link
Contributor

Incorporating the speedups detailed in the "Speeding up Percolator" JPR paper. The pull restructures nested CV SVM training so that only a single OMP threadpool is used (a large portion of the overhaul takes place in doStep() within CrossValidation.cpp). The memory footprint using multiple threads in this new implementation is also much lighter than that shown in the paper (the single OMP threadpool is much more memory efficient and I fixed a memory leak in L2-SVM-MFN).

Copy link
Collaborator

@MatthewThe MatthewThe left a comment

Choose a reason for hiding this comment

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

Thanks again for the pull request! Overall it looks good, the major obstacle is probably support for compilers that do not support OpenMP, but hopefully that can be solved with a couple of well-placed ifdefs.

Copy link
Collaborator

@MatthewThe MatthewThe left a comment

Choose a reason for hiding this comment

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

Looking good! A few changes are still needed for our builds with Visual Studio 2015 and I found some other minor issues as well.

Also, please merge the latest changes from the master branch which contain some fixes for our Windows and OSX builds.

@johnhalloran321
Copy link
Contributor Author

Thanks for reviewing the code, Matthew! The only change I couldn't do was removing the C array in ssl.cpp; removing in ssl.cpp, ssl.h, and Scores.cpp resulted in zero identified PSMs. It seems to require more work to figure out what's happening with this unnecessary array, but all the other changes are checked in and I just merged with the main Percolator master branch.

Copy link
Collaborator

@MatthewThe MatthewThe left a comment

Choose a reason for hiding this comment

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

Found two more issues. Also, could you replace your tabs by spaces, this will solve some of the weird indentation.

@johnhalloran321
Copy link
Contributor Author

Just checked in the fix for nested-xvals and removed all tabs. Let me know if I missed any files with weird indentations and I can untabify them.

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