Conf sampling #757

JintaoWu98 · 2024-07-15T05:44:09Z

Updated generate_conformers and deduce_new_conformers to output hypothetical_num_comb.
Refined determine_torsion_symmetry to handle carbon rotors with three identical attachments, improving symmetry accuracy.
Adjusted all references in the codebase to align with these changes, ensuring consistency.

alongd · 2024-07-16T15:51:16Z

arc/species/conformers.py

+            for angle in torsion_scan:
+                test_angle = (angle + 60) % 360
+                if any((test_angle - 5) % 360 <= existing_angle <= (test_angle + 5) % 360 for existing_angle in torsion_scan):
+                    for angle in torsion_scan:


@JintaoWu98, see the CodeQL comment for this line

codecov · 2024-07-15T06:39:05Z

Codecov Report

Attention: Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Project coverage is 73.72%. Comparing base (0d16383) to head (e87e6c8).

Files	Patch %	Lines
arc/species/conformers.py	95.83%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #757      +/-   ##
==========================================
+ Coverage   73.69%   73.72%   +0.02%     
==========================================
  Files          99       99              
  Lines       27432    27454      +22     
  Branches     5773     5783      +10     
==========================================
+ Hits        20217    20240      +23     
+ Misses       5765     5764       -1     
  Partials     1450     1450

Flag	Coverage Δ
unittests	`73.72% <95.83%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alongd · 2024-07-16T15:51:16Z

arc/species/conformers.py

+            for angle in torsion_scan:
+                test_angle = (angle + 60) % 360
+                if any((test_angle - 5) % 360 <= existing_angle <= (test_angle + 5) % 360 for existing_angle in torsion_scan):
+                    for angle in torsion_scan:


@JintaoWu98, see the CodeQL comment for this line

alongd · 2024-07-16T15:54:25Z

arc/species/conformers.py

@@ -298,9 +300,9 @@ def generate_conformers(mol_list: Union[List[Molecule], Molecule],
        lowest_confs, new_conformers = list(), list()

    if not return_all_conformers:
-        return lowest_confs
+        return lowest_confs, hypothetical_num_comb


Do we need to return hypothetical_num_comb for the production code, or just for our internal study?

arc/species/conformers_test.py

+        new_conformers, symmetries, _ = conformers.deduce_new_conformers(
+                label='', conformers=confs, torsions=torsions, tops=tops, mol_list=[spc1.mol], plot_path=None,
+                combination_threshold=10, force_field='MMFF94s', max_combination_iterations=25)


alongd

Thanks, please see some comments

alongd · 2024-07-20T19:57:10Z

arc/species/conformers.py

@@ -341,7 +341,7 @@ def deduce_new_conformers(label, conformers, torsions, tops, mol_list, smeared_s
    symmetries = dict()
    for torsion, top in zip(torsions, tops):
        # identify symmetric torsions so we don't bother considering them in the conformational combinations
-        symmetry = determine_torsion_symmetry(label, top, mol_list, torsion_angles[tuple(torsion)])
+        symmetry, torsion_angles[tuple(torsion)] = determine_torsion_symmetry(label, top, mol_list, torsion_angles[tuple(torsion)])


Above we do torsion_angles = get_torsion_angles(label, conformers, torsions)
and here we overwrite torsion_angles[tuple(torsion)]. Can you explain this change?

To address this comment and the next one, I define a new function add_torsion_symmetry to update the torsion_angle if needed (currently for rotors like CH3 or CF3).

alongd · 2024-07-20T19:57:41Z

arc/species/conformers.py

@@ -995,7 +1014,7 @@ def determine_torsion_symmetry(label, top1, mol_list, torsion_scan):
                elif not mol.atoms[top[0] - 1].lone_pairs > 0 and not mol.atoms[top[0] - 1].radical_electrons > 0 \
                        and all([groups[0].is_isomorphic(group, save_order=True) for group in groups[1:]]):
                    symmetry *= len(groups)
-    return symmetry
+    return symmetry, torsion_scan


please consider again whether we need to return torsion_scan

I define a new function add_torsion_symmetry to resolve this issue.

alongd · 2024-07-20T19:57:54Z

arc/species/conformers.py

    """
    symmetry = 1
    check_tops = [1, 1]  # flags for checking top1 and top2
    mol = mol_list[0]
    top2 = [i + 1 for i in range(len(mol.atoms)) if i + 1 not in top1]
    for j, top in enumerate([top1, top2]):
-        # A quick bypass for methyl rotors which are too common:
+


please remove the empty line

Thanks, it's fixed.

alongd · 2024-07-20T19:58:35Z

arc/species/conformers.py

    """
    symmetry = 1
    check_tops = [1, 1]  # flags for checking top1 and top2
    mol = mol_list[0]
    top2 = [i + 1 for i in range(len(mol.atoms)) if i + 1 not in top1]
    for j, top in enumerate([top1, top2]):
-        # A quick bypass for methyl rotors which are too common:
+
+        # A quick bypass for carbon rotors which has three equal elements with only one bond that is connected to the carbon atom


typo, should be that have instead of which has
Add to the comment: e.g., CH3 or CF3

Thanks, it's fixed.

arc/species/conformers.py

alongd · 2024-07-20T20:03:22Z

arc/species/conformers.py

+            new_angles = []
+            for angle in torsion_scan:
+                test_angle = (angle + 60) % 360
+                if any((test_angle - 5) % 360 <= existing_angle <= (test_angle + 5) % 360 for existing_angle in torsion_scan):


test_angle was already defined with % 360 above, can you explain why we test % 360 again here?

If angle in line 945 is 299, test_angle is then 359, thus we need it in line 947 since test_angle+5 will be larger than 360.

alongd

Thanks! I added a few more comments

alongd · 2024-07-21T06:07:07Z

arc/species/conformers.py

@@ -341,6 +341,7 @@ def deduce_new_conformers(label, conformers, torsions, tops, mol_list, smeared_s
    symmetries = dict()
    for torsion, top in zip(torsions, tops):
        # identify symmetric torsions so we don't bother considering them in the conformational combinations
+        torsion_angles[tuple(torsion)] = add_torsion_symmetry(top, mol_list, torsion_angles[tuple(torsion)])


Maybe the call to add_torsion_symmetry() should be done within get_torsion_angles(), so we generate it correctly in the first place?

You are right, I moved it.

arc/species/conformers.py

alongd · 2024-07-21T06:11:51Z

arc/species/conformers.py

+    Returns:
+        torsion_scan (list): The angles corresponding to this torsion from all conformers.
+    """
+


Can you remove this extra line?

alongd · 2024-07-21T06:13:22Z

arc/species/conformers.py

+        torsion_scan (list): The angles corresponding to this torsion from all conformers.
+
+    Returns:
+        torsion_scan (list): The angles corresponding to this torsion from all conformers.


This should be different than the description on line 1012, please describe briefly how torsion_scan is modified

alongd · 2024-07-21T06:19:21Z

arc/species/conformers.py

+                        # Check if the new angle, adjusted by ±30 degrees, overlaps with any existing angles
+                        if not any((new_angle - 30) % 360 <= existing_angle <= (new_angle + 30) % 360 for existing_angle in torsion_scan + new_angles):
+                            new_angles.append(new_angle)
+                    break


Perhaps the break should be under the if statement? This way, once the first angle is added, we'll stop the loop and be more efficient, we don't need another one at the same location

alongd · 2024-07-21T06:21:41Z

arc/species/conformers.py

@@ -998,6 +1002,46 @@ def determine_torsion_symmetry(label, top1, mol_list, torsion_scan):
    return symmetry


+def add_torsion_symmetry(top1, mol_list, torsion_scan):


I suggest to rename this function. The current name sounds like we take the torsions and add the symmetry number to the (e.g., symmetry=3)
What about add_missing_symmetric_torsion_values() or something similar?

This can prevent extra symmetries when `symmetry` is multiple times of well number

It will detect and handle carbon rotor that has three equal atoms. Define `add_torsion_symmetry` for adding symmetry to a torsion scan to account for efficient conformer generation.

`determine_torsion_symmetry` can now handle carbon rotor that has three equal atoms only attached to itself.

alongd

Looking good, thanks!

github-actions bot added Module: Conformers Module: Species labels Jul 15, 2024

github-advanced-security bot found potential problems Jul 15, 2024

View reviewed changes

alongd reviewed Jul 16, 2024

View reviewed changes

JintaoWu98 force-pushed the conf_sampling branch from 462cc1f to d73201a Compare July 19, 2024 05:03

github-advanced-security bot found potential problems Jul 19, 2024

View reviewed changes

JintaoWu98 force-pushed the conf_sampling branch from d73201a to 1cd352e Compare July 19, 2024 05:10

alongd reviewed Jul 20, 2024

View reviewed changes

JintaoWu98 force-pushed the conf_sampling branch from 1cd352e to 0d37efa Compare July 21, 2024 01:25

alongd reviewed Jul 21, 2024

View reviewed changes

JintaoWu98 force-pushed the conf_sampling branch from 0d37efa to c9f4aaa Compare July 21, 2024 08:17

JintaoWu98 added 3 commits July 21, 2024 16:25

Modify the symmetry logic

6404e37

This can prevent extra symmetries when `symmetry` is multiple times of well number

Effective symmetry angle sampling method

8aa985b

It will detect and handle carbon rotor that has three equal atoms. Define `add_torsion_symmetry` for adding symmetry to a torsion scan to account for efficient conformer generation.

Handle carbon rotor with three equal atoms for symmetry determination

e87e6c8

`determine_torsion_symmetry` can now handle carbon rotor that has three equal atoms only attached to itself.

JintaoWu98 force-pushed the conf_sampling branch from c9f4aaa to e87e6c8 Compare July 21, 2024 08:28

alongd approved these changes Jul 21, 2024

View reviewed changes

alongd merged commit 08e7e93 into main Jul 21, 2024
7 checks passed

alongd deleted the conf_sampling branch July 21, 2024 13:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conf sampling #757

Conf sampling #757

JintaoWu98 commented Jul 15, 2024

alongd Jul 16, 2024

codecov bot commented Jul 15, 2024 •

edited

Loading

alongd Jul 16, 2024

alongd Jul 16, 2024

alongd left a comment

alongd Jul 20, 2024

JintaoWu98 Jul 21, 2024 •

edited

Loading

alongd Jul 20, 2024

JintaoWu98 Jul 21, 2024 •

edited

Loading

alongd Jul 20, 2024

JintaoWu98 Jul 21, 2024

alongd Jul 20, 2024

JintaoWu98 Jul 21, 2024 •

edited

Loading

alongd Jul 20, 2024

JintaoWu98 Jul 21, 2024 •

edited

Loading

alongd left a comment

alongd Jul 21, 2024

JintaoWu98 Jul 21, 2024

alongd Jul 21, 2024

alongd Jul 21, 2024

alongd Jul 21, 2024

alongd Jul 21, 2024

alongd left a comment

		@@ -998,6 +1002,46 @@ def determine_torsion_symmetry(label, top1, mol_list, torsion_scan):
		return symmetry


		def add_torsion_symmetry(top1, mol_list, torsion_scan):

Conf sampling #757

Conf sampling #757

Conversation

JintaoWu98 commented Jul 15, 2024

Choose a reason for hiding this comment

codecov bot commented Jul 15, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alongd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JintaoWu98 Jul 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JintaoWu98 Jul 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JintaoWu98 Jul 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JintaoWu98 Jul 21, 2024 • edited Loading

Choose a reason for hiding this comment

alongd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alongd left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 15, 2024 •

edited

Loading

JintaoWu98 Jul 21, 2024 •

edited

Loading

JintaoWu98 Jul 21, 2024 •

edited

Loading

JintaoWu98 Jul 21, 2024 •

edited

Loading

JintaoWu98 Jul 21, 2024 •

edited

Loading