-
Notifications
You must be signed in to change notification settings - Fork 2
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
Replace PyRosetta with OpenMM in Fragmenstein #1078
Comments
Get Steph's use-case and see if there's a work-around. Bug OpenMM community about the (1) funny numbers and (2) very slow running. To finalise the ticket: |
She said there was no issue in the Fragmenstein side
I could not get the calculated values to behave nor cut down the speed, it all works fine, but the values on testing are terrible. The values are a bit hairy, but the sign is somewhat consistent. Even with PyRosetta the interactions were the main driver and the results get filtered by bad energy and sorted by interactions primarily at least in my hands. Bad:
Good:
|
@matteoferla when is your RDkit fix going into production? (@Waztom add to agenda for Thursday.) |
Unless the Squonk job has a frozen version of fragmenstein (doubtful), it should have been in production since last month (i.e. version 0.13.20) @tdudgeon —Is this true? And if not, could the version of Fragmenstein be changed to the latest |
@tdudgeon there's question for you from @matteoferla, in the question above. If this is indeed deployed, then the ticket can move to "In production." |
Currently Fragmenstein is used for 2 sets of jobs:
Both use the same base container that currently uses Fragmenstein 0.12.0 (https://github.com/InformaticsMatters/squonk2-fragmenstein/blob/main/requirements.txt#L2) Updating this is simple and will just need the container images to be rebuilt. But are there other impacts? If this means that the updated jobs would now use OpenMM for minimisation (rather that Pyrosetta (originally) or nothing (more recently)) do we need some extra options to allow this minimisation to be enabled and/or configured? |
Thank you for the link: that is super helpful! No, they will not use OpenMM for minimisation —rigid backbone only. Custom options For the PyRosetta, the academic username and password are universal, so we could conceivably have an input box wherein the user provides the password which if its hash is correct runs the job. For some demo I did it that way. Future I should say that there is another ticket, wherein I need to Squonkificate the normal Fragmenstein pipeline (bunk double or triple mergers, analog-by-catalog search, placement, clustering and ranking), but I need to change the analog-by-catalog search by copying Steph's FragmentKnitting via FragmentNetwork as oppose to use sw.docking.org. Steph is busy so I have been trying to reverse engineer it. I will talk to Rubén about how the Squonkification of Steph's code. |
So Fragmenstein version should be increased to 0.13.34, without any other changes being needed? |
@matteoferla your answers don't look relevant to this ticket; if you need them actioned, please create new ticket. I suspect they pertain to Brownish release. As for "custom options": no need for a ticket, the answer is categorical: there will be no PyRosetta in Fragalysis, or any support for custom licenses, certainly not in the forseeable future - we don't have the legal backup to make this happen. |
@phraenquex Apologies. I will try to be more clear in future |
@tdudgeon @matteoferla can I move this into Production swimlane now? If not, what are the remaining actions? |
I still need to update the fragmenstein version and deploy. |
I should have asked: when is that happening? Is it a lot of work - or can it happen now, without slowing down green release? @matteoferla you're on standby to test it, presumably? |
@phraenquex my help stops at saying to change this line to |
Yes, I just need to make the change and rebuild the images. Just not had time to do so yet. |
@tdudgeon that hasn't answered my question. @matteoferla no ticket is complete until someone knowledgeable has confirmed that it "works". You're the only one that could credibly do that, at the moment. Unless you show @mwinokan how to do it. |
@phraenquex: yes, as soon as @tdudgeon signals that the image rebuild is complete I will test it. |
The fragmenstein images have now been rebuilt, and should be testable in Squonk. Steph's fragment knitting images need to be rebuilt by @rsanchezgarc as I can't push to the XChem DockerHub repo. @phraenquex No, it shouldn't be moved to production until the above has been done. But really this isn't much to do with Fragalysis staging/production/etc as it's purely Squonk related, and Squonk will use the new images as soon as they become available. |
Thanks @tdudgeon. @rsanchezgarc could you give an indication of when this can happen? (If you can't do it, we'll ask @mwinokan or @Waztom to take it on.) Correction: They are absolutely staging/production related: the categories refer to what's available to users, not to some arcane technical details about repos and stuff. They always have. |
@tdudgeon @matteoferla . Shouldn't we also change the line of code in which we set up Victor or Wictor or whatever the new openmm class is called? |
@rsanchezgarc —OpenVictor was a no go. OpenMM energy minimisation is very slow —it happens fully in cartesian space without internal space tricks and akin to an MD run at zero kelvin with a shaking sampler to add motion as far as I can tell. Template choice becomes critical in Wictor (without PyRosetta) as the neighbourhood is frozen. So RDKit is the only choice here. |
@matteoferla .Then, how do you use it? Just as our old |
@tdudgeon. I already had |
If the job hasn't changed (only the fragmenstein implementation) then all that's needed is a new container image. |
@tdudgeon so it should be ready now |
@tdudgeon thinks it was already done, needs to check - please do for green release. |
@tdudgeon can you confirm these changes were made? We will need to revisit the deployed algorithms in general in the orange release |
The jobs are currently using Fragmenstein version 0.13.36 which I believe is one that no longer uses PyRosetta, but @matteoferla should confirm this. |
Thank you for confirming @tdudgeon |
@tdudgeon — I can confirm that is correct. As you said, Fragmenstein can be run
The code implemented in Fragalysis only does a merger, which is likely not in catalogue space. |
Thanks @matteoferla. We will revisit the deployed algorithms in a later release (see #1454), and I will get in touch regarding the analogue search (although we quite like the 'pure' merges for CAR anyway) |
@mwinokan, great, using the unaltered mergers for the in-house synthesis pipeline makes total sense: jumping into catalogue space is infuriating (eg. heteroarenes to benzenes and loss of substituents) and that is even with the fact that SW runs not on FPs but on edit distance —the Astex Fragment network is an open copycat but with fewer of Roger and John's mathemagical tricks. 🤷 |
@phraenquex I think it's best to address it in #1454 |
Sure - in which case, we're probably better off moving this ticket to #won'tfix - life being too short, and there being loads of loose ends with the overall problem anyway. |
No description provided.
The text was updated successfully, but these errors were encountered: