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

bug-frame-charge-94 #95

Merged
merged 8 commits into from
May 2, 2024
Merged

bug-frame-charge-94 #95

merged 8 commits into from
May 2, 2024

Conversation

universvm
Copy link
Member

Closes #94 Fix

ChrisWellsWood
ChrisWellsWood previously approved these changes Apr 17, 2024
Copy link
Contributor

@ChrisWellsWood ChrisWellsWood left a comment

Choose a reason for hiding this comment

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

Looks good, approved. One thought on this that we should discuss though, your threshold for the Zimmerman polarity is quite high, so only a few residues are counted as polar, and this doesn't include residues like serine or threonine, which are traditionally included as such. We might want to explore using Grantham polarity with a cut off around 8 too.

@sunal1996
Copy link
Contributor

sunal1996 commented Apr 22, 2024

In this part of create_frame_dataset.py

            gaussian_atom = gaussian_matrix[:, :, :, atom_idx]
            # Add at position:
            frame = add_gaussian_at_position(
                main_matrix=frame,
                secondary_matrix=gaussian_atom,
                atom_coord=indices,
                atom_idx=atom_idx,
            )
            if res_property != 0:
                gaussian_atom = gaussian_matrix[:, :, :, atom_idx] * float(res_property)
                # Add at position:
                frame = add_gaussian_at_position(
                    main_matrix=frame,
                    secondary_matrix=gaussian_atom,
                    atom_coord=indices,
                    atom_idx=5,
                    normalize=False,
                )
                

Does it appear that we are declaring the gaussian_atom variable and running add_gaussian_at_position function redundantly? In that case, does this one make sense:

            if res_property != 0:
               gaussian_atom = gaussian_matrix[:, :, :, atom_idx] * float(res_property)
               # Add at position:
               frame = add_gaussian_at_position(
                   main_matrix=frame,
                   secondary_matrix=gaussian_atom,
                   atom_coord=indices,
                   atom_idx=5,
                   normalize=False,
             else:
                gaussian_atom = gaussian_matrix[:, :, :, atom_idx]
                  # Add at position:
                frame = add_gaussian_at_position(
                     main_matrix=frame,
                     secondary_matrix=gaussian_atom,
                     atom_coord=indices,
                     atom_idx=atom_idx,
                    )

@universvm
Copy link
Member Author

@sunal1996 in the case of a charged residue (say lysine), the gaussian needs to be added in the atom index (C, N, O, Ca, Cb) AND in the charge index (ie. the fifth).

I think in your if/else logic all the atoms of a charged residue (C, N, O, Ca, Cb) would end up in the 5th channel and there would be no atoms in the C, N, O, Ca, Cb channels

@sunal1996
Copy link
Contributor

Ah, all clear.

Copy link
Contributor

@ChrisWellsWood ChrisWellsWood left a comment

Choose a reason for hiding this comment

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

Looks good merge when ready.

@ChrisWellsWood ChrisWellsWood merged commit e6c86b0 into master May 2, 2024
4 checks passed
@ChrisWellsWood ChrisWellsWood deleted the bug-frame-charge-94 branch May 2, 2024 16:48
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.

Charge Frames are either all positive or all negative
3 participants