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

minimal edits to get faster fragmenting #10

Merged
merged 3 commits into from
Aug 8, 2022
Merged

minimal edits to get faster fragmenting #10

merged 3 commits into from
Aug 8, 2022

Conversation

pstjohn
Copy link
Collaborator

@pstjohn pstjohn commented Aug 1, 2022

@IchiruTake, do these edits come close to what you've implemented without completely changing the fragment API? My timing comparisons have them as fairly close

@IchiruTake
Copy link

IchiruTake commented Aug 1, 2022

@pstjohn Yes, if you said by algorithm it is matched but by your algorithm, we are lacking control of the molecule. The class molecule make me nervous of multiple states across. If I add both smiles and mol that are not the same, they are still valid either.

Moreover, I have extensively improved the documentation to make sure for further customization. Take the 5 status of the bond I opened in the class MolProcessor or the updated documentation of BDFE in predict.py

It would be great to have stricter control and a systematic approach for what user should do rathen pushing every possibilities.

Overall, this PR has done it great tuning job, but lack of further feature and variable control for beeter memory management.

@IchiruTake
Copy link

I have reviewed your fragment.py which allowed further molecule control

@pstjohn pstjohn merged commit fce4496 into master Aug 8, 2022
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