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

Hydra-Powered YAML Configuration and Enhanced File Support in HelixFold3 #321

Closed
wants to merge 42 commits into from

Conversation

YaoYinYing
Copy link

@YaoYinYing YaoYinYing commented Aug 20, 2024

Added

  • Configurability via YAML, supported by Hydra and OmegaConf.
  • Support for SDF file inputs for small molecules and additional file formats.
  • Expanded set of demo cases.
  • New helixfold run command for performing inference.
  • Covalent Bonding support from ligand to protein residue.
  • Disulfide Bonding

Changed

  • Transitioned HelixFold3 to a pip-installable package.
  • Overrides now handled through CONFIG_DIFFS.
  • Replaced ml_collections.ConfigDict with DictConfig for better integration.

Fixed

  • Enhanced support for MSA parallel processing and full BFD.
  • Implemented automatic binary search within the PATH if not explicitly specified.
  • Resolved an issue with dummy assertions of ref_atom_name_chars and problematic atoms that caused errors.
  • Addressed the None-value in TemplateAtomMaskAllZerosError to prevent incorrect warning triggers.

Removed

  • Internal build-in logger to streamline output management.
  • Deprecated requirements.txt in favor of direct pip installation dependencies.
  • Removed run_infer.sh script as functionality is now integrated into the main command interface.

@CLAassistant
Copy link

CLAassistant commented Aug 20, 2024

CLA assistant check
All committers have signed the CLA.

@Fairly Fairly requested review from RyanGarciaLI and removed request for RyanGarciaLI August 26, 2024 08:01
@Fairly Fairly self-assigned this Aug 26, 2024
Copy link
Collaborator

@Fairly Fairly left a comment

Choose a reason for hiding this comment

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

Thank you for your interest in our work and for the new features you've provided! Before we review your PR, we kindly ask that you split the implemented features into different PRs based on their categories. This will ensure that each PR contains minimal changes, making it easier for us to track code modifications. Once again, thank you for your contribution!

@YaoYinYing
Copy link
Author

YaoYinYing commented Aug 27, 2024

@Fairly Sure! The biggest problem to me however, is splitting those commits into several PRs simutanously would create huge load of conflict solving issues.

Any solutions for this? Or can I just create a single PR each time for you to revise and merge? That may take long but would be simplest.

@Fairly
Copy link
Collaborator

Fairly commented Aug 27, 2024

Creating a single PR each time is fine. Please split these commits based on functionality; for example, changes related to configuration files (Hydra & OmegaConf) can be placed in one PR, while support for covalent bond inputs can be in another. Bug fixes can be distributed across different PRs, so there's no need to be particularly concerned about keeping them organised. Hope these work isn't too much of a burden for you~

@YaoYinYing
Copy link
Author

YaoYinYing commented Aug 27, 2024

@Fairly Great! I have reordered these commits and did some stashings to reduce the total number of commits that you guys need to revise. I will create 3 PRs in total, with each contains 4-6 commits.

The full branch can be found here.

Now, here's the PR roadmap:

id purpose # commits Affected PR
1 Hydra-Omegaconf and Pip module 6 Code, Config, Doc #325
2 BFD supports and MSA parallelism fixes 5 Code, Config, Case
3 Small molecule inputs and covalent bonds 4 Code, Case, Doc #329

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.

4 participants