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

Dimer update #14

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

ChenouZhang
Copy link
Contributor

Rolled back from #11 to separate the updates for GridData and drawmembrane2a.c.
Muted dimer option so that old code still works. Implementation of command line passing of dimer parameters is needed for next PR.

Updated Author list.

…rs are hard coded(see comments in draw_membrane2a. In the future, we need to add those parameters to the BornProfiler workflow.)
Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Let's get this PR to the stage where draw_membrane2a

  • has new command line options to generate a symmetric exclusion zone
  • has documentation for the new options
  • will by default work as before

As the next steps, can you write in a comment what you envisage the usage for draw_membrane2a should look like for your case: what additional data do you need to provide to the program in order for it to generate the correct dimer exclusion zones?

/* bilayer or headgroup dielectric constant outside channel */
*diel = (z >= M->z_h0 && z <= M->z_h1) ? M->mdie : M->idie;
if (R2 <= R_temp || R <= R_temp){
//if (R2 <= R_temp) { // test the other protomer
Copy link
Member

Choose a reason for hiding this comment

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

remove commented out code


/* if dimer */
else {
R = sqrt(SQR(x-M->x0_R) + SQR(y-M->y0_R)); /*better to use the Membrane struct for x2, y2*/
Copy link
Member

Choose a reason for hiding this comment

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

What does the comment mean?


/* Draw symmetric exclusion zone for a dimer. */
/* The zone is defined by x0_R, y0_R, dx_R, dy_R. */
bool dimer = FALSE; /* New feature for CPA dimer. Set to FALSE as default*/
Copy link
Member

Choose a reason for hiding this comment

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

Change formatting and comment.

Suggested change
bool dimer = FALSE; /* New feature for CPA dimer. Set to FALSE as default*/
bool dimer=FALSE; /* dimeric proteins with symmetric exclusion zones. Set to FALSE as default*/

Comment on lines +552 to +555
/* For this test case, we are going to hard code the second zone center as:*/
/* x0_R - 2 * dx_R */
/* y0_R - 2 * dy_R */
/* see core.py line 458*/
Copy link
Member

Choose a reason for hiding this comment

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

Is this going to be replaced by an alternative way to set the position?

If so, what is needed as input to the program?

Copy link
Member

Choose a reason for hiding this comment

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

Or should this be deleted??

Comment on lines +585 to +589
/* Warning: this is the hard coded offset. Normally you should get them from core.py*/
/* But the way wrote in core.py made it hard to do so. */
dx_R = 20;
dy_R = -5;

Copy link
Member

Choose a reason for hiding this comment

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

Needs to be replaced with input.

@@ -770,10 +819,21 @@ int main(int argc, char *argv[])
Membrane.x0_p = x0_p; /* centre of the protein */
Membrane.y0_p = y0_p;
Membrane.z0_p = z0_p;
Membrane.dimer = False; /*CPA hack*/
Copy link
Member

Choose a reason for hiding this comment

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

Remove the comment... assuming that this value will be set from somewhere?


/* Manually add parameters for the second protomer. */
if (dimer){
printf("Dimer system detected. Turning on the hack mode.");
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call it "symmetric dimer" mode? "Hack" sounds as if it's not really supposed to be here.

@@ -770,10 +819,21 @@ int main(int argc, char *argv[])
Membrane.x0_p = x0_p; /* centre of the protein */
Membrane.y0_p = y0_p;
Membrane.z0_p = z0_p;
Membrane.dimer = False; /*CPA hack*/

/* Manually add parameters for the second protomer. */
Copy link
Member

Choose a reason for hiding this comment

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

WHat is meant by the comment? Why "manually"?

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