MDSPACE #153
Merged
MDSPACE #153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MohamadHarastani
approved these changes
Jan 30, 2023
continuousflex/protocols.conf
Outdated
| {"tag": "protocol", "value": "FlexProtNMA", "text": "NMA"} | ||
| ]}, | ||
| {"tag": "section", "text": "6. MDSPACE", "children": [ | ||
| {"tag": "protocol", "value": "ProtMDSPACE", "text": "MDSPACE", "icon": "bookmark.png"} |
There was a problem hiding this comment.
You may also add the PDB dimred, grouping etc.
| @@ -164,10 +167,9 @@ def _defineParams(self, form): | |||
| "to accelerate NM integration, however can make the simulation unstable.", | |||
| condition="simulationType==2 or simulationType==4",expertLevel=params.LEVEL_ADVANCED) | |||
| group.addParam('nm_mass', params.FloatParam, default=10.0, label='NM mass', | |||
| help="Mass value of Normal modes for NMMD", condition="simulationType==2 or simulationType==4", | |||
| help="Mass value of Normal modes for NMMD. Lower values accelerate the fitting but can make the " | |||
| "simulation unstable", condition="simulationType==2 or simulationType==4", | |||
There was a problem hiding this comment.
We are all guilty, but it is better if we extract constants instead of using numbers of choices in simulationType==2 or simulationType==4
There was a problem hiding this comment.
You are right, this is an old code I need to update it, in new code I do it
| inputAlignement = self._createSetOfVolumes("inputAlignement") | ||
| readSetOfVolumes(self.getAlignementprefix(), inputAlignement) | ||
| alignedSet = self._createSetOfVolumes("alignedSet") | ||
| # if self.EMfitChoice.get() == EMFIT_VOLUMES: |
There was a problem hiding this comment.
If there is something to do here, better to keep a comment why this section is commented, otherwise, clean it.
| writeSetOfVolumes(alignedSet, self.getAlignementprefix()) | ||
| else: | ||
| writeSetOfParticles(alignedSet, self.getAlignementprefix()) | ||
| # if isinstance(inputSet, SetOfVolumes): |
There was a problem hiding this comment.
same comment as above
| @@ -581,6 +582,27 @@ def generate_rotation_and_shift(self): | |||
| else: | |||
| psi1 = np.random.normal(self.MeanPsi.get(), self.StdPsi.get()) | |||
|
|
|||
| # uniform over the sphere | |||
There was a problem hiding this comment.
I found a direct way to get 3 Euler angles with uniform distribution using:
from scipy.spatial.transform import Rotation
rotation = Rotation.random() # this give a random rotational vector
rot, tilt, psi = rotation.as_euler('ZYZ', degrees=True)
There was a problem hiding this comment.
This make me actually wonder if we need to keep the "Gaussian distribution" options at all. It seems to be complicating things and we have never used it.
There was a problem hiding this comment.
That is indeed much easier with scipy!
Yes... I believe it is useless we should remove it.
I am not sure that choosing the range of the angles is useful as well, I never changed this range, maybe we should just give the possibility to set the range of translation and for rotation just set yes or no you want to synthesize the rotations
There was a problem hiding this comment.
I agree. Shifts make sense to be simulated in Gaussian distribution around zero since the person (or algorithm) tries to hit the center. But angles have no meaning like that. There are a couple of complex patterns that I have noticed in preferred orientations, I will think how to implement that later (probably an interactive tool).
|
Hi Remi |
|
You asked me about the plotter with 100,000. Absolutely, you did it in a perfect way! |
|
Hi Remi |
This error is definitely the error related to gcc <= 9, that is weird that you have it with gcc 10 |
|
I was testing on a fresh install of Ububtu20 on WSL. I will tell you what was my version once I am back home. I was following the steps of scipion documentation for the installation. I installed gcc-10 but maybe I did not update alternatives to use it. |
| @@ -402,7 +382,7 @@ def _validate(self): | |||
| return errors | |||
|
|
|||
| def _citations(self): | |||
| return ['harastani2022continuousflex','vuillemot2022NMMD'] | |||
| return ['harastani2022continuousflex','vuillemot2022NMMD','vuillemot2023mdspace'] | |||
There was a problem hiding this comment.
You may actually want to revert the order of these citations.
Yes, in any case my code was suppose to handle any gcc version so there is a problem how I implemented it, I'll work on it. You 'll let me know the output of gcc --version and gfortran --version |
|
The ones that failed I had gcc 9.4.0 and gfortran 9.4.0 |
|
Okay then there is a problem with how I read the version. I do that : for me with gfortran 9.40 it gives me 9. |
|
Actually this gave me the same number! I changed now gcc to 10. I am testing the fixes that I have done for the installation on master, I will be back and test if that solved my problem on this branch. |
|
The installation of this branch went well! I don't know if you changed something or my rv_genesis branch was not updated at the time I tested. But now it installs. |
|
I recently compile xmipp, they must have refactored or just removed this binary xmipp_metadata_selfile_create |
|
It exists in python Here is the script https://github.com/I2PC/xmipp/blob/devel/src/xmipp/legacy/applications/scripts/metadata_selfile_create/batch_metadata_selfile_create.py#L29 |
|
I know I had the same issue when updating Xmipp. It appears they removed a lot of things among which xmipp_metadata_selfile_create. We need to update a lot of protocols |
|
A nightmare! I will see what I can do, but we have no choice :-) |
|
Hi @mms29 |
|
I need to merge this PR and solve conflicts with the other one |
|
Great! Thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New protocol viewer and tests for MDSPACE
fix of PDB dim reduction problems
fix issues for GENESIS installation in init.py
fix a bug in protocol_image_synthesize (the output was not pointing to the right images files but the "subtomograms.vol" probably some copy/paste from synthesize subtomo). Also, when reading the input PDBs, you did not sort them by the name resulting in random order of input PDBs which is not the best. for now I just sort the list of PDB, but it will be solved once we allow to input a SetOfPDBs
added the uniform distribution of angles around the sphere in protocol_subtomograms_synthesize (copied the code I've done previously in protocol_image_synthesize)
I modified the nma_plotter to limit the number of point displayed to 100,000 (This will affect all protocol using it). The thing is that the viewer does not respond with plots with more than 100,000 points, it gets so slow you can not do anything. Here I just limit the display but it does not affect the clustering tool, for example you can select a region of point and it will recognize all the points inside even the ones that are not displayed. I believe that beyond 100,000 points, you do not distinguish individual points so it does not matter if you have only 100,000, you still see the overall distribution. What do you think ?