Re: [Denovoassembler-devel] git diff kmersmatrix branch
Ray -- Parallel genome assemblies for parallel DNA sequencing
Brought to you by:
sebhtml
From: Sébastien B. <se...@bo...> - 2014-03-07 02:18:39
|
Hi Maxime, I can't merge your code in master without making some changes first. As the maintainer, here are the changes that I will need to do to increase the plus-value of your work (on your next pull request, you can think about some of these points if you want): Keep in mind that Coding style is very important for readability. (you can check https://github.com/sebhtml/ray/blob/master/Documentation/CodingStyle.txt ) Here are the changes that I will make tomorrow: - (C0) As discussed, we don't want to generate Surveyor/KmerMatrix.tsv by default because this is quite large file. There needs to be an option. - (C1) Obviously, if you want people to use your Surveyor workflow, you need to add documentation. You just need to add some code in code/Mock/Parameters.cpp. After that you build Ray, then you write ./Ray -help> MANUAL_PAGE.txt - (C2) KmerMatrixOwner.h/cpp was done by you. So the copyright in the header belongs to you, not me. - (C2) there is some red showing up in 'git diff --color', nothing important though relevant commits ce6c272 & 4e4949 - (C3) Copyright for code/Surveyor/SequenceKmerReader.cpp/.h should be in 2014 - (C4) In code/Surveyor/StoreKeeper.h you must use tabulations and no spaces for indentation. for example, the line with MERGE_KMER_MATRIX uses spaces in StoreKeeper.h - (C5) code/Surveyor/KmerMatrixOwner.cpp was using spaces instead of tabulations. - (C6) the method name KmerMatrixOwner::printLocalKmersMatrix is meaningless as it does not write a matrix, it writes a kmer. - (C7) you hard-coded the kmer value (31) !!! m_kmerMatrix << kmer.idToWord(31,0); you must avoid that ! [KmerMatrixOwner.cpp] - (C8) The FIRST_TAG for your reader is the same that is used by the graph reader. I think we said it was OK as long as you added a comment about the reason. code/Surveyor/GenomeAssemblyReader.h: FIRST_TAG = 10200, code/Surveyor/GenomeGraphReader.h: FIRST_TAG = 10200, Keep up the great work, but next time, I think you can make progress on respecting the coding style, among other things. I will do a bunch of commits in my branch patch-kmermatrix (fetched from remotes/zorino/patch-kmermatrix) to address the comments above. [boiseb01@ls30 ray]$ git diff master..remotes/zorino/patch-kmermatrix --stat code/Surveyor/GenomeAssemblyReader.cpp | 3 +- code/Surveyor/KmerMatrixOwner.cpp | 157 ++++++++++++++++++++ code/Surveyor/KmerMatrixOwner.h | 72 +++++++++ code/Surveyor/Makefile | 1 + code/Surveyor/MatrixOwner.cpp | 19 +-- code/Surveyor/MatrixOwner.h | 3 +- code/Surveyor/Mother.cpp | 253 +++++++++++++++++++++---------- code/Surveyor/Mother.h | 17 ++- code/Surveyor/SequenceKmerReader.cpp | 53 ++++++- code/Surveyor/SequenceKmerReader.h | 2 + code/Surveyor/StoreKeeper.cpp | 117 +++++++++++---- code/Surveyor/StoreKeeper.h | 20 +++- 12 files changed, 580 insertions(+), 137 deletions(-) Link: http://sourceforge.net/p/denovoassembler/mailman/denovoassembler-devel/thread/017C19ECDB98F64F99200D83876C44520111A7FC4FE7%40EXCH-MBX-B.ulaval.ca/#msg31999514 Link: http://sourceforge.net/p/denovoassembler/mailman/denovoassembler-devel/thread/COL131-W760CDDC5071A415AB9C5EDAC870%40phx.gbl/#msg32015211 Link: http://sourceforge.net/p/denovoassembler/mailman/denovoassembler-devel/thread/f2d8f53ef133555e36529047f4e95084%40boisvert.info/#msg31993547 Link: http://sourceforge.net/p/denovoassembler/mailman/denovoassembler-devel/thread/5301F42B.6060904%40gmail.com/#msg31988453 ---------------------------------------- > Date: Wed, 5 Mar 2014 13:41:39 +0000 > From: max...@gm... > To: se...@bo... > Subject: Re: [Denovoassembler-devel] git diff kmersmatrix branch > > Hi Sebastien, > > I have fix the issue when scaffolds would be given in entry instead of > contigs in the SequenceKmerReader class. > > I also made the edition according to this review. > > Please now pull from : > > https://github.com/Zorino/ray.git > > patch-kmermatrix > > Cheers, > > Maxime > > > > On 02/23/2014 12:16 PM, Sébastien Boisvert wrote: >> Hey Maxime, >> >> You did not provide KmersMatrixOwner.h, KmersMatrixOwner.cpp, and changes >> to the Surveyor Makefile. >> >> >> OTher comments are below. >> >> ---------------------------------------- >>> Date: Sat, 22 Feb 2014 10:03:30 +0000 >>> From: ma...@de... >>> To: se...@bo... >>> Subject: git diff kmersmatrix branch >>> >>> diff --git a/code/Surveyor/MatrixOwner.cpp b/code/Surveyor/MatrixOwner.cpp >>> index ffaae00..47cf84a 100644 >>> --- a/code/Surveyor/MatrixOwner.cpp >>> +++ b/code/Surveyor/MatrixOwner.cpp >>> @@ -65,9 +65,12 @@ void MatrixOwner::receive(Message & message) { >>> assert(m_parameters != NULL); >>> assert(m_sampleNames != NULL); >>> #endif >>> - >>> m_mother = source; >>> >>> + //open the buffer of the file >>> + // createKmersMatrixOutputFile(); >>> + >>> + >>> } else if(tag == PUSH_PAYLOAD) { >>> >>> SampleIdentifier sample1 = -1; >>> @@ -89,10 +92,10 @@ void MatrixOwner::receive(Message & message) { >>> assert(count>= 0); >>> #endif >>> >>> - /* >>> + >>> printName(); >>> - cout << "DEBUG add " << sample1 << " " << sample2 << " " >>> << count << endl; >>> -*/ >>> + // cout << "DEBUG add " << sample1 << " " << sample2 << >> Commented lines should be removed. >> >>> " " << count << endl; >>> + >>> m_receivedPayloads ++; >>> >>> m_localGramMatrix[sample1][sample2] += count; >>> @@ -100,14 +103,14 @@ void MatrixOwner::receive(Message & message) { >>> Message response; >>> response.setTag(PUSH_PAYLOAD_OK); >>> send(source, response); >>> + } >>> + else if(tag == PUSH_PAYLOAD_END) { >> Use '} else if (' and not '} >> else if' >> >> >> This is the coding style of the project. >> see https://github.com/sebhtml/ray/blob/master/Documentation/CodingStyle.txt >> >> * Kernighan and Ritchie style, variant "The One True Brace Style" (1TBS) >> http://en.wikipedia.org/wiki/Indent_style#Variant:_1TBS >> >> >> (you used K&R Variant: Stroustrup). >> >>> - } else if(tag == PUSH_PAYLOAD_END) { >>> - >>> + cout << "PUSH_PAYLOAD_END" <<endl; >> Remove this debug message. >> >>> m_completedStoreActors++; >>> >>> if(m_completedStoreActors == getSize()) { >>> >>> - >>> printName(); >>> cout << "MatrixOwner received " << >>> m_receivedPayloads << " payloads" << endl; >>> >>> @@ -151,10 +154,9 @@ void MatrixOwner::receive(Message & message) { >>> >>> >>> // tell Mother that the matrix is ready now. >>> - >>> - Message coolMessage; >>> - coolMessage.setTag(MATRIX_IS_READY); >>> - send(m_mother, coolMessage); >>> + Message coolMessage; >>> + coolMessage.setTag(GRAM_MATRIX_IS_READY); >>> + send(m_mother, coolMessage); >>> >>> >>> // clear matrices >>> @@ -275,3 +277,4 @@ void >>> MatrixOwner::printLocalGramMatrixWithHash(ostream & stream, map<SampleIdent >>> stream << endl; >>> } >>> } >>> + >>> diff --git a/code/Surveyor/MatrixOwner.h b/code/Surveyor/MatrixOwner.h >>> index ceb17e2..afa9278 100644 >>> --- a/code/Surveyor/MatrixOwner.h >>> +++ b/code/Surveyor/MatrixOwner.h >>> @@ -28,6 +28,7 @@ >>> >>> #include <map> >>> #include <iostream> >>> +#include <sstream> >>> using namespace std; >>> >>> class MatrixOwner : public Actor { >>> @@ -62,7 +63,7 @@ public: >>> PUSH_PAYLOAD, >>> PUSH_PAYLOAD_OK, >>> PUSH_PAYLOAD_END, >>> - MATRIX_IS_READY, >>> + GRAM_MATRIX_IS_READY, >>> LAST_TAG >>> }; >>> >>> diff --git a/code/Surveyor/Mother.cpp b/code/Surveyor/Mother.cpp >>> index 4d2ef9c..103a583 100644 >>> --- a/code/Surveyor/Mother.cpp >>> +++ b/code/Surveyor/Mother.cpp >>> @@ -27,6 +27,7 @@ >>> #include "GenomeGraphReader.h" >>> #include "GenomeAssemblyReader.h" >>> #include "MatrixOwner.h" >>> +#include "KmersMatrixOwner.h" >>> >>> #include <RayPlatform/cryptography/crypto.h> >>> >>> @@ -39,11 +40,13 @@ using namespace std; >>> #define INPUT_TYPE_GRAPH 0 >>> #define INPUT_TYPE_ASSEMBLY 1 >>> >>> - >>> Mother::Mother() { >>> >>> m_coalescenceManager = -1; >>> m_matrixOwner = -1; >>> + m_kmersMatrixOwner = -1; >>> + >>> + // m_matricesAreReady = true; >> Remove this commented line. >> >>> m_parameters = NULL; >>> m_bigMother = -1; >>> @@ -91,7 +94,7 @@ void Mother::receive(Message & message) { >>> notifyController(); >>> } >>> >>> - } else if(tag == MERGE) { >>> + } else if(tag == MERGE_GRAM_MATRIX) { >>> >>> int matrixOwner = -1; >>> memcpy(&matrixOwner, buffer, sizeof(matrixOwner)); >>> @@ -102,7 +105,7 @@ void Mother::receive(Message & message) { >>> #endif >>> >>> Message theMessage; >>> - theMessage.setTag(StoreKeeper::MERGE); >>> + theMessage.setTag(StoreKeeper::MERGE_GRAM_MATRIX); >>> theMessage.setBuffer(&matrixOwner); >>> theMessage.setNumberOfBytes(sizeof(matrixOwner)); >>> >>> @@ -111,10 +114,33 @@ void Mother::receive(Message & message) { >>> send(destination, theMessage); >>> >>> Message response; >>> - response.setTag(MERGE_OK); >>> + response.setTag(MERGE_GRAM_MATRIX_OK); >>> + send(source, response); >>> + >>> + } else if (tag == MERGE_KMERS_MATRIX) { >>> + >>> + int kmersMatrixOwner = -1; >>> + memcpy(&kmersMatrixOwner, buffer, sizeof(kmersMatrixOwner)); >>> + >>> +#ifdef CONFIG_ASSERT >>> + assert(kmersMatrixOwner>= 0); >>> + assert(m_storeKeepers.size() == 1); >>> +#endif >>> + >>> + Message theMessage; >>> + theMessage.setTag(StoreKeeper::MERGE_KMERS_MATRIX); >> The name should be MERGE_KMER_MATRIX and not MERGE_KMERS_MATRIX. >> >>> + theMessage.setBuffer(&kmersMatrixOwner); >>> + theMessage.setNumberOfBytes(sizeof(kmersMatrixOwner)); >>> + >>> + int destination = m_storeKeepers[0]; >>> + >>> + send(destination, theMessage); >>> + >>> + Message response; >>> + response.setTag(MERGE_KMERS_MATRIX_OK); >>> send(source, response); >>> >>> - } else if(tag == SHUTDOWN) { >>> + } else if(tag == SHUTDOWN) { >>> >>> Message response; >>> response.setTag(SHUTDOWN_OK); >>> @@ -122,18 +148,16 @@ void Mother::receive(Message & message) { >>> >>> stop(); >>> >>> - } else if(tag == StoreKeeper::MERGE_OK) { >>> + } else if(tag == StoreKeeper::MERGE_GRAM_MATRIX_OK) { >>> >>> // TODO: the bug https://github.com/sebhtml/ray/issues/216 >>> // is caused by the fact that this message is not >>> // received . >>> >>> - /* >>> - Message newMessage; >>> - newMessage.setTag(MERGE_OK); >>> + // Message newMessage; >>> + // newMessage.setTag(MERGE_OK); >>> >>> - send(m_bigMother, newMessage); >>> - */ >>> + // send(m_bigMother, newMessage); >> Remove these commented lines. >> >>> } else if(tag == FINISH_JOB) { >>> >>> @@ -153,6 +177,7 @@ void Mother::receive(Message & message) { >>> >>> sendToFirstMother(FLUSH_AGGREGATOR, >>> FLUSH_AGGREGATOR_RETURN); >>> } >>> + >>> } else if(tag == FLUSH_AGGREGATOR) { >>> >>> /* >>> @@ -188,64 +213,52 @@ void Mother::receive(Message & message) { >>> cout << "DEBUG sending FLUSH_AGGREGATOR_OK to >>> m_bigMother" << endl; >>> */ >>> >>> - } else if(tag == MatrixOwner::MATRIX_IS_READY) { >>> + } else if(tag == MatrixOwner::GRAM_MATRIX_IS_READY) { >>> + >>> + //TODO : check if all matrices are ready >>> + if(m_matricesAreReady){ >>> + sendToFirstMother(SHUTDOWN, SHUTDOWN_OK); >>> + }else { >>> + cout << "GRAM_MATRIX_IS_READY" << endl; >> When an actor speak, you must print its name too in stdout. >> (with printName()). >> >>> + m_matricesAreReady = true; >>> + } >>> >>> - sendToFirstMother(SHUTDOWN, SHUTDOWN_OK); >>> + } >>> + else if(tag == KmersMatrixOwner::KMERS_MATRIX_IS_READY) { >>> + >>> + cout << "KMERS_MATRIX_IS_READY" << endl; >>> + if(m_matricesAreReady){ >>> + sendToFirstMother(SHUTDOWN, SHUTDOWN_OK); >>> + }else { >>> + cout << "KMERS_MATRIX_IS_READY" << endl; >>> + m_matricesAreReady = true; >> >> In one comment above, I saw that m_matricesAreReady = false >> was commented. Check that out. >> >>> + } >>> >>> } else if(tag == FLUSH_AGGREGATOR_OK) { >>> >>> - /* >>> printName(); >>> cout << "DEBUG received FLUSH_AGGREGATOR_OK" << endl; >>> - */ >>> >>> m_flushedMothers++; >>> >>> if(m_flushedMothers < getSize()) >>> return; >>> >>> - // spawn the MatrixOwner here ! >>> - >>> - MatrixOwner * matrixOwner = new MatrixOwner(); >>> - spawn(matrixOwner); >>> - >>> - m_matrixOwner = matrixOwner->getName(); >>> - >>> - printName(); >>> - cout << "Spawned MatrixOwner actor !" << endl; >>> - >>> - // tell the StoreKeeper actors to send their stuff to the >>> - // MatrixOwner actor >>> - // The Mother of Mother will wait for a signal from >>> MatrixOwner >>> - >>> - Message greetingMessage; >>> - >>> - vector<string> * names = & m_sampleNames; >>> - >>> - char buffer[32]; >>> - int offset = 0; >>> - memcpy(buffer + offset, &m_parameters, >>> sizeof(m_parameters)); >>> - offset += sizeof(m_parameters); >>> - memcpy(buffer + offset, &names, sizeof(names)); >>> - offset += sizeof(names); >>> - >>> - greetingMessage.setBuffer(&buffer); >>> - greetingMessage.setNumberOfBytes(offset); >>> - >>> - greetingMessage.setTag(MatrixOwner::GREETINGS); >>> - send(m_matrixOwner, greetingMessage); >>> - >>> - sendToFirstMother(MERGE, MERGE_OK); >>> - >>> + spawnMatrixOwner(); >> I like that. A method to spawn an actor. Good ! >> >>> } else if(tag == m_responseTag) { >>> >>> - >>> if(m_responseTag == SHUTDOWN_OK) { >>> >>> - } else if(m_responseTag == MERGE_OK) { >>> - >>> - } else if(m_responseTag == FLUSH_AGGREGATOR_RETURN) { >>> + } else if(m_responseTag == MERGE_GRAM_MATRIX_OK) { >>> + // All mothers merged their GRAM MATRIX >>> + // Spawn KmersMatrixOwner to print >>> + if(m_motherToKill < getSize() && >>> m_printKmersMatrix){ >>> + spawnKmersMatrixOwner(); >>> + } >>> + } else if(m_responseTag == MERGE_KMERS_MATRIX_OK) { >>> + } >>> + else if(m_responseTag == FLUSH_AGGREGATOR_RETURN) { >> Again, put closing brace on same line (} else if ( ...) { >> >>> /* >>> printName(); >>> @@ -254,9 +267,8 @@ void Mother::receive(Message & message) { >>> */ >>> } >>> >>> - // every mother was informed. >>> + // every mother was not informed. >> Good catch ! >> >>> if(m_motherToKill>= getSize()) { >>> - >>> sendMessageWithReply(m_motherToKill, m_forwardTag); >>> m_motherToKill--; >>> } >>> @@ -284,11 +296,15 @@ void Mother::sendMessageWithReply(int & actor, int >>> tag) { >>> Message message; >>> message.setTag(tag); >>> >>> - if(tag == MERGE) { >>> + if(tag == MERGE_GRAM_MATRIX) { >>> message.setBuffer(&m_matrixOwner); >>> message.setNumberOfBytes(sizeof(m_matrixOwner)); >>> - >>> - } else if(tag == FLUSH_AGGREGATOR) { >>> + } >>> + else if(tag == MERGE_KMERS_MATRIX) { >>> + message.setBuffer(&m_kmersMatrixOwner); >>> + message.setNumberOfBytes(sizeof(m_kmersMatrixOwner)); >>> + } >>> + else if(tag == FLUSH_AGGREGATOR) { >>> >>> /* >>> printName(); >>> @@ -328,6 +344,10 @@ void Mother::stop() { >>> m_matrixOwner = -1; >>> } >>> >>> + if(m_kmersMatrixOwner>= 0) { >>> + send(m_kmersMatrixOwner, kill); >>> + m_kmersMatrixOwner = -1; >>> + } >>> >>> die(); >>> >>> @@ -410,39 +430,44 @@ void Mother::startSurveyor() { >>> >>> bool isRoot = (getName() % getSize()) == 0; >>> >>> - //cout << "DEBUG startSurveyor isRoot" << isRoot << endl; >>> - >>> - // get a list of files. >>> + // Set matricesAreReady to true in case user doesn't want >>> + // to print out kmers matrix. >>> + m_matricesAreReady = true; >>> >>> vector<string> * commands = m_parameters->getCommands(); >>> >>> - >>> for(int i = 0 ; i < (int) commands->size() ; ++i) { >>> >>> string & element = commands->at(i); >>> >>> - // DONE: Check bounds for file names >>> + if (element != "-print-kmers-matrix") { >> The name should be kmer-matrix, not kmers-matrix. >> >> It is like groceries store vs grocery store. >> >>> + // DONE: Check bounds for file names >>> >>> - map<string,int> fastTable; >>> + map<string,int> fastTable; >>> >>> - fastTable["-read-sample-graph"] = INPUT_TYPE_GRAPH; >>> - fastTable["-read-sample-assembly"] = INPUT_TYPE_ASSEMBLY; >>> + fastTable["-read-sample-graph"] = INPUT_TYPE_GRAPH; >>> + fastTable["-read-sample-assembly"] = >>> INPUT_TYPE_ASSEMBLY; >>> >>> - // Unsupported option >>> - if(fastTable.count(element) == 0 || i+2> (int) >>> commands->size()) >>> - continue; >>> + // Unsupported option >>> + if(fastTable.count(element) == 0 || i+2> (int) >>> commands->size()) >>> + continue; >>> >>> - string sampleName = commands->at(++i); >>> - string fileName = commands->at(++i); >>> + string sampleName = commands->at(++i); >>> + string fileName = commands->at(++i); >>> >>> - m_sampleNames.push_back(sampleName); >>> + m_sampleNames.push_back(sampleName); >>> >>> - // DONE implement this m_assemblyFileNames + type >>> - m_inputFileNames.push_back(fileName); >>> + // DONE implement this m_assemblyFileNames + type >>> + m_inputFileNames.push_back(fileName); >>> >>> - int type = fastTable[element]; >>> + int type = fastTable[element]; >>> >>> - m_sampleInputTypes.push_back(type); >>> + m_sampleInputTypes.push_back(type); >>> + >>> + } else { >>> + m_matricesAreReady = false; >>> + m_printKmersMatrix = true; >> >> Question: if m_printKmersMatrix is false, I suppose the code >> follows the usual path of printing just one matrix, right ? >> >>> + } >>> >>> } >>> >>> @@ -468,6 +493,9 @@ void Mother::startSurveyor() { >>> >>> m_storeKeepers.push_back(actor->getName()); >>> >>> + actor->setOutputKmersMatrixPath(m_parameters->getPrefix()); >> The path should be prefix/Surveyor/<whatever the kmer matrix's name is> >> >>> + actor->setSamplesSize(m_sampleNames.size()); >> sample size, not samples size. >> >>> + >>> // tell the CoalescenceManager about the local StoreKeeper >>> Message dummyMessage; >>> int localStore = actor->getName(); >>> @@ -568,6 +596,80 @@ void Mother::spawnReader() { >>> } >>> } >>> >>> + >>> +void Mother::spawnMatrixOwner() { >>> + >>> + // spawn the MatrixOwner here ! >>> + MatrixOwner * matrixOwner = new MatrixOwner(); >>> + spawn(matrixOwner); >>> + >>> + m_matrixOwner = matrixOwner->getName(); >>> + >>> + printName(); >>> + cout << "Spawned MatrixOwner actor !" << m_matrixOwner << endl; >>> + >>> + // tell the StoreKeeper actors to send their stuff to the >>> + // MatrixOwner actor >>> + // The Mother of Mother will wait for a signal from MatrixOwner >>> + >>> + Message greetingMessage; >>> + >>> + vector<string> * names = & m_sampleNames; >>> + >>> + char buffer[32]; >>> + int offset = 0; >>> + memcpy(buffer + offset, &m_parameters, sizeof(m_parameters)); >>> + offset += sizeof(m_parameters); >>> + memcpy(buffer + offset, &names, sizeof(names)); >>> + offset += sizeof(names); >>> + >>> + greetingMessage.setBuffer(&buffer); >>> + greetingMessage.setNumberOfBytes(offset); >>> + >>> + greetingMessage.setTag(MatrixOwner::GREETINGS); >>> + send(m_matrixOwner, greetingMessage); >>> + >>> + sendToFirstMother(MERGE_GRAM_MATRIX, MERGE_GRAM_MATRIX_OK); >>> +} >>> + >>> +void Mother::spawnKmersMatrixOwner() { >>> + >>> + // spawn the MatrixOwner here ! >>> + KmersMatrixOwner * kmersMatrixOwner = new KmersMatrixOwner(); >>> + spawn(kmersMatrixOwner); >>> + >>> + m_kmersMatrixOwner = kmersMatrixOwner->getName(); >>> + >>> + printName(); >>> + cout << "Spawned KmersMatrixOwner actor !" << >>> m_kmersMatrixOwner << endl; >>> + >>> + // tell the StoreKeeper actors to send their stuff to the >>> + // KmersMatrixOwner actor >>> + // The Mother of Mother will wait for a signal from MatrixOwner >>> + >>> + Message greetingMessage; >>> + >>> + vector<string> * names = & m_sampleNames; >>> + >>> + char buffer[32]; >>> + int offset = 0; >>> + memcpy(buffer + offset, &m_parameters, sizeof(m_parameters)); >>> + offset += sizeof(m_parameters); >>> + memcpy(buffer + offset, &names, sizeof(names)); >>> + offset += sizeof(names); >>> + >>> + greetingMessage.setBuffer(&buffer); >>> + greetingMessage.setNumberOfBytes(offset); >>> + >>> + greetingMessage.setTag(KmersMatrixOwner::GREETINGS); >>> + send(m_kmersMatrixOwner, greetingMessage); >>> + >>> + sendToFirstMother(MERGE_KMERS_MATRIX, MERGE_KMERS_MATRIX_OK); >>> + >>> +} >>> + >>> + >>> void Mother::setParameters(Parameters * parameters) { >>> m_parameters = parameters; >>> } >>> + >>> diff --git a/code/Surveyor/Mother.h b/code/Surveyor/Mother.h >>> index 092920f..9774c4b 100644 >>> --- a/code/Surveyor/Mother.h >>> +++ b/code/Surveyor/Mother.h >>> @@ -28,6 +28,7 @@ >>> >>> #include <vector> >>> #include <string> >>> +#include <iostream> >>> using namespace std; >>> >>> /** >>> @@ -55,9 +56,12 @@ class Mother: public Actor { >>> private: >>> >>> int m_matrixOwner; >>> + int m_kmersMatrixOwner; >>> >>> int m_flushedMothers; >>> int m_finishedMothers; >>> + bool m_matricesAreReady; >>> + bool m_printKmersMatrix; >>> >>> Parameters * m_parameters; >>> >>> @@ -93,6 +97,13 @@ private: >>> */ >>> void sendToFirstMother(int forwardTag, int responseTag); >>> >>> + /* int m_kmersMatrixBlocNumber; */ >>> + void printLocalKmersMatrix(string & kmer, string & >>> samples_kmers, bool force); >>> + void createKmersMatrixOutputFile(); >>> + >>> + void spawnMatrixOwner(); >>> + void spawnKmersMatrixOwner(); >>> + >> That's a good design -- private methods for private uses. >> >>> public: >>> >>> Mother(); >>> @@ -109,8 +120,10 @@ public: >>> FLUSH_AGGREGATOR, >>> FLUSH_AGGREGATOR_OK, >>> FLUSH_AGGREGATOR_RETURN, >>> - MERGE, >>> - MERGE_OK, >>> + MERGE_GRAM_MATRIX, >>> + MERGE_GRAM_MATRIX_OK, >>> + MERGE_KMERS_MATRIX, >>> + MERGE_KMERS_MATRIX_OK, >> kmer matrix, not kmers matrix. >> >>> LAST_TAG, >>> }; >>> >>> diff --git a/code/Surveyor/StoreKeeper.cpp b/code/Surveyor/StoreKeeper.cpp >>> index 84eef34..492208c 100644 >>> --- a/code/Surveyor/StoreKeeper.cpp >>> +++ b/code/Surveyor/StoreKeeper.cpp >>> @@ -22,10 +22,16 @@ >>> #include "StoreKeeper.h" >>> #include "CoalescenceManager.h" >>> #include "MatrixOwner.h" >>> +#include "KmersMatrixOwner.h" >>> >>> #include <code/VerticesExtractor/Vertex.h> >>> +#include <RayPlatform/structures/MyHashTableIterator.h> >>> +#include <RayPlatform/core/OperatingSystem.h> >>> >>> #include <iostream> >>> +#include <sstream> >>> +#include <iomanip> >>> +#include <fstream> >>> using namespace std; >>> >>> #include <string.h> >>> @@ -83,15 +89,21 @@ void StoreKeeper::receive(Message & message) { >>> >>> die(); >>> >>> - } else if(tag == MERGE) { >>> + } else if(tag == MERGE_GRAM_MATRIX) { >>> >>> >>> - printName(); >>> - cout << "DEBUG at MERGE message reception "; >>> - cout << "(StoreKeeper) received " << m_receivedObjects >>> << " objects in total"; >>> - cout << " with " << m_receivedPushes << " push >>> operations" << endl; >>> + // printName(); >>> + // cout << "DEBUG at MERGE_GRAM_MATRIX message reception "; >>> + // cout << "(StoreKeeper) received " << >>> m_receivedObjects << " objects in total"; >>> + // cout << " with " << m_receivedPushes << " push >> You can remove commented lines. >> >>> operations" << endl; >>> computeLocalGramMatrix(); >>> >>> + >>> + // TODEL Print matrix bloc >>> + // m_kmersMatrixBlocNumber = 0; >>> + // printLocalKmersMatrix(); >>> + >> You can remove commented lines. >> >>> + >>> m_mother = source; >>> >>> memcpy(&m_matrixOwner, buffer, sizeof(m_matrixOwner)); >>> @@ -108,19 +120,32 @@ void StoreKeeper::receive(Message & message) { >>> m_iterator2 = m_iterator1->second.begin(); >>> } >>> >>> - /* >>> - printName(); >>> - cout << "DEBUG printLocalGramMatrix before first >>> sendMatrixCell" << endl; >>> - printLocalGramMatrix(); >>> - */ >>> - >>> + // printName(); >>> + // cout << "DEBUG printLocalGramMatrix before first >>> sendMatrixCell" << endl; >>> + // printLocalGramMatrix(); >> You can remove commented lines. >> >>> sendMatrixCell(); >>> >>> } else if(tag == MatrixOwner::PUSH_PAYLOAD_OK) { >>> - >>> sendMatrixCell(); >>> + } else if(tag == MERGE_KMERS_MATRIX) { >>> + // cout << "DEBUG at MERGE_GRAM_MATRIX message reception "; >>> + // cout << "(StoreKeeper) received " << >>> m_receivedObjects << " objects in total"; >>> + // cout << " with " << m_receivedPushes << " push >>> operations" << endl; >> You can remove commented lines. >> >> Otherwise, add a "#ifdef DEBUG_SOMETHING_SOMETHING / #endif around that lines". >> >> >>> + >>> + m_mother = source; >>> >>> - } else if(tag == CoalescenceManager::SET_KMER_LENGTH) { >>> + memcpy(&m_kmersMatrixOwner, buffer, >>> sizeof(m_kmersMatrixOwner)); >>> + >>> + m_hashTableIterator.constructor(&m_hashTable); >>> + >>> + sendKmersSamples(); >>> + } >>> + else if (tag == KmersMatrixOwner::PUSH_KMER_SAMPLES_END) { >>> + } >>> + else if(tag == KmersMatrixOwner::PUSH_KMER_SAMPLES_OK) { >>> + sendKmersSamples(); >>> + } >>> + else if(tag == CoalescenceManager::SET_KMER_LENGTH) { >>> >>> int kmerLength = 0; >>> int position = 0; >>> @@ -181,8 +206,6 @@ void StoreKeeper::sendMatrixCell() { >>> message.setNumberOfBytes(offset); >>> message.setTag(MatrixOwner::PUSH_PAYLOAD); >>> >>> - //cout << " DEBUG send PUSH_PAYLOAD to " << >>> m_matrixOwner << endl; >>> - >>> send(m_matrixOwner, message); >>> >>> m_iterator2++; >>> @@ -207,10 +230,7 @@ void StoreKeeper::sendMatrixCell() { >>> // free memory. >>> m_localGramMatrix.clear(); >>> >>> - /* >>> printName(); >>> - cout << "DEBUG send PUSH_PAYLOAD_END to " << m_matrixOwner << endl; >>> - */ >>> >>> Message response; >>> response.setTag(MatrixOwner::PUSH_PAYLOAD_END); >>> @@ -236,6 +256,7 @@ void StoreKeeper::configureHashTable() { >>> ); >>> >>> m_configured = true; >>> + >>> } >>> >>> void StoreKeeper::printColorReport() { >>> @@ -375,6 +396,7 @@ void StoreKeeper::computeLocalGramMatrix() { >>> //printLocalGramMatrix(); >>> } >>> >>> + >>> void StoreKeeper::printLocalGramMatrix() { >>> >>> printName(); >>> @@ -623,3 +645,73 @@ void StoreKeeper::storeData(Vertex & vertex, int & >>> sample) { >>> >>> */ >>> } >>> + >>> + >>> +void StoreKeeper::setSamplesSize(int sampleSize) { >>> + m_sampleSize = sampleSize; >>> +} >>> + >>> +void StoreKeeper::setOutputKmersMatrixPath(string pathPrefix) { >>> + // m_outputKmersMatrixPath = pathPrefix; >>> + // m_outputKmersMatrixPath += "/KmersMatrixDump/"; >>> + // createDirectory(m_outputKmersMatrixPath.c_str()); >> You can remove commented lines. >> >> >> This file could be in prefix/Surveyor/<...> >> >>> +} >>> + >>> + >>> +void StoreKeeper::sendKmersSamples() { >>> + >>> + char buffer[4000]; >> For portability, use MAXIMUM_MESSAGE_SIZE_IN_BYTES instead of 4000. >> >>> + int bytes = 0; >>> + >>> + ExperimentVertex * currentVertex = NULL; >>> + VirtualKmerColorHandle currentVirtualColor = NULL_VIRTUAL_COLOR; >>> + >>> + vector<bool> samplesVector (m_sampleSize, false); >>> + >>> + if(m_hashTableIterator.hasNext()){ >>> + >>> + // fill(samplesVector.begin(),samplesVector.end(),false); >>> + >> You can remove commented lines. >> >>> + currentVertex = m_hashTableIterator.next(); >>> + Kmer kmer = currentVertex->getKey(); >>> + >>> + bytes += kmer.dump(buffer); >>> + >>> + currentVirtualColor = currentVertex->getVirtualColor(); >>> + set<PhysicalKmerColor> * samples = >>> m_colorSet.getPhysicalColors(currentVirtualColor); >>> + >>> + for(set<PhysicalKmerColor>:: iterator sampleIterator = >>> samples->begin(); >>> + sampleIterator != samples->end(); ++sampleIterator) { >>> + PhysicalKmerColor value = *sampleIterator; >>> + samplesVector[value] = true; >>> + // cout << " " << value; >>> + } >>> + >>> + for (std::vector<bool>::iterator it = >>> samplesVector.begin(); >>> + it != samplesVector.end(); ++it) { >>> + buffer[bytes] = *it; >>> + bytes++; >>> + } >>> + // buffer[bytes] = '\0'; >> You can remove commented lines. >> >>> + } >>> + >>> + >>> + Message message; >>> + message.setNumberOfBytes(bytes); >>> + message.setBuffer(buffer); >>> + >>> + // message.setTag(MatrixOwner::PUSH_KMERS_SAMPLES); >>> + if(m_hashTableIterator.hasNext()){ >>> + message.setTag(KmersMatrixOwner::PUSH_KMER_SAMPLES); >>> + }else{ >>> + message.setTag(KmersMatrixOwner::PUSH_KMER_SAMPLES_END); >>> + } >>> + >>> + // Message response; >>> + // response.setTag(MatrixOwner::PUSH_PAYLOAD_END); >>> + // send(m_matrixOwner, response); >>> + >>> + send(m_kmersMatrixOwner, message); >>> + >>> +} >>> + >>> diff --git a/code/Surveyor/StoreKeeper.h b/code/Surveyor/StoreKeeper.h >>> index e44cf98..36adf77 100644 >>> --- a/code/Surveyor/StoreKeeper.h >>> +++ b/code/Surveyor/StoreKeeper.h >>> @@ -34,6 +34,10 @@ >>> >>> #include <RayPlatform/actors/Actor.h> >>> #include <RayPlatform/structures/MyHashTable.h> >>> +#include <RayPlatform/structures/MyHashTableIterator.h> >>> + >>> +#include <iostream> >>> +#include <sstream> >>> >>> /** >>> * Provides genomic storage. >>> @@ -55,6 +59,7 @@ private: >>> >>> int m_mother; >>> int m_matrixOwner; >>> + int m_kmersMatrixOwner; >>> >>> bool m_configured; >>> >>> @@ -64,6 +69,8 @@ private: >>> */ >>> MyHashTable<Kmer,ExperimentVertex> m_hashTable; >>> >>> + MyHashTableIterator<Kmer,ExperimentVertex> m_hashTableIterator; >>> + >>> int m_kmerLength; >>> bool m_colorSpaceMode; >>> >>> @@ -79,6 +86,13 @@ private: >>> void printLocalGramMatrix(); >>> void printColorReport(); >>> >>> + /* ostringstream m_currentKmer; */ >>> + /* ostringstream m_currentSamplesKmers; */ >>> + int m_sampleSize; >>> + string m_outputKmersMatrixPath; >>> + void printLocalKmersMatrix(string & m_kmer, string & >>> m_samplesKmers); >>> + void sendKmersSamples(); >>> + >>> void sendMatrixCell(); >>> >>> public: >>> @@ -86,14 +100,19 @@ public: >>> StoreKeeper(); >>> ~StoreKeeper(); >>> >>> + void setOutputKmersMatrixPath(string pathPrefix); >>> + void setSamplesSize(int sampleSize); >>> + >>> void receive(Message & message); >>> >>> enum { >>> FIRST_TAG = 10250, >>> PUSH_SAMPLE_VERTEX, >>> PUSH_SAMPLE_VERTEX_OK, >>> - MERGE, >>> - MERGE_OK, >>> + MERGE_GRAM_MATRIX, >>> + MERGE_GRAM_MATRIX_OK, >>> + MERGE_KMERS_MATRIX, >>> + MERGE_KMERS_MATRIX_OK, >>> LAST_TAG >>> }; >>> }; >> ------------------------------------------------------------------------------ >> Managing the Performance of Cloud-Based Applications >> Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. >> Read the Whitepaper. >> http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk >> _______________________________________________ >> Denovoassembler-devel mailing list >> Den...@li... >> https://lists.sourceforge.net/lists/listinfo/denovoassembler-devel > |