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

Added molecular system reader #92

Merged
merged 4 commits into from
Dec 21, 2016
Merged

Added molecular system reader #92

merged 4 commits into from
Dec 21, 2016

Conversation

favreau
Copy link
Member

@favreau favreau commented Dec 20, 2016

The MolecularSystemReader class loads proteins and meshes according to parameters defined in a Molecular System Configuration file. This configuration is defined by the following entries:

  • ProteinFolder: Folder containing pdb files
  • MeshFolder: Folder containing obj files
  • SystemDescriptor: File containing the IDs of the proteins
  • ProteinPositions: File containing the position of each protein

size_t id;
size_t instances;
lineStream >> protein >> id >> instances;
if( protein == "" )
Copy link
Contributor

Choose a reason for hiding this comment

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

.empty()

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


proteins[id] = protein;

if( proteinFolder == "" )
Copy link
Contributor

Choose a reason for hiding this comment

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

.empty()

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

}
descriptorFile.close();

// Load proteins according to specified positions
Copy link
Contributor

Choose a reason for hiding this comment

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

for readability you could move the comment-marked blocks to private methods. Would also 'solve' the file.close() at the end of each block.

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, working on it

if( proteins.find(id) != proteins.end( ))
{
// Scale to correct units
position *= 10.f;
Copy link
Contributor

Choose a reason for hiding this comment

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

where does that magic come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

Some files are in angstrom, others in nanometers... Ask the data providers ;-)

* @return bounding box of the protein
* @param PDB file to import
* @param Position of protein in space
* @param Index of the protein when more than one is loaded
Copy link
Contributor

Choose a reason for hiding this comment

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

doxygen warns here for not finding those param names I suppose :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link

@rdumusc rdumusc left a comment

Choose a reason for hiding this comment

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

+1 otherwise

boost::filesystem::path fileExtension =
dirIter->path( ).extension( );
if( fileExtension==".swc" || fileExtension==".h5" )
boost::filesystem::path fileExtension = dirIter->path().extension();
Copy link

Choose a reason for hiding this comment

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

move inside the else{} as it is not used outside, can be const auto

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

MorphologyLoader morphologyLoader( geometryParameters );

strings filters = { ".swc", ".h5" };
strings files = _parseFolder( folder, filters );
Copy link

Choose a reason for hiding this comment

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

const strings (multiple occurences)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

std::cout << std::endl;\
__logging_mtx.unlock();\
} \
__logging_mtx.lock();\
Copy link

Choose a reason for hiding this comment

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

Just a detail, but in general it's a good habit to use RAII style:
std::lock_guard<std::mutex> lock(__logging_mtx);

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Thanks.

public:

/**
* @brief Default constructor
Copy link

Choose a reason for hiding this comment

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

not the default since it takes a parameter :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@tribal-tec tribal-tec left a comment

Choose a reason for hiding this comment

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

Something seems weird with the commits included in this PR. Rebase maybe?

@favreau
Copy link
Member Author

favreau commented Dec 21, 2016

I know, I did something wrong again. I'll squash them and rebase accordingly

@favreau favreau merged commit 4a3071b into BlueBrain:master Dec 21, 2016
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.

None yet

3 participants