Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #695 +/- ##
==========================================
+ Coverage 73.80% 73.97% +0.17%
==========================================
Files 99 101 +2
Lines 27352 27596 +244
Branches 5718 5746 +28
==========================================
+ Hits 20187 20415 +228
- Misses 5738 5745 +7
- Partials 1427 1436 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
alongd
force-pushed
the
linear_ts
branch
5 times, most recently
from
8825e9f to
cbec920
Compare
Also fixed the is_isomerization() method
Also testing the atom_order arg
| print(self.rxn_2.ts_species.ts_guesses[0].initial_xyz) | ||
| self.assertEqual(self.rxn_2.ts_species.ts_guesses[0].initial_xyz['symbols'], | ||
| ('C', 'C', 'O', 'N', 'O', 'H', 'H', 'H', 'H', 'H')) | ||
| expected_xyz = 1 # todo |
Check notice
Code scanning / CodeQL
Unused local variable Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, to fix an unused local variable you either (1) remove the variable assignment if it is unnecessary, ensuring you preserve any side effects on the right-hand side, or (2) rename it to an "unused" style name (for example, _ or unused_expected_xyz) if it is intentionally unused. Here, the right-hand side is the literal 1, which has no side effects, and the variable is not used. The cleanest fix without changing functionality is to delete the expected_xyz assignment but retain the # todo comment so the intent is preserved.
Concretely, in arc/job/adapters/ts/linear_test.py inside test_linear_adapter_2, remove line 1167 (expected_xyz = 1 # todo) and leave everything else unchanged. No imports, methods, or additional definitions are needed.
| @@ -1164,7 +1164,7 @@ | ||
| print(self.rxn_2.ts_species.ts_guesses[0].initial_xyz) | ||
| self.assertEqual(self.rxn_2.ts_species.ts_guesses[0].initial_xyz['symbols'], | ||
| ('C', 'C', 'O', 'N', 'O', 'H', 'H', 'H', 'H', 'H')) | ||
| expected_xyz = 1 # todo | ||
| # todo: add assertion on expected_xyz once reference geometry is defined | ||
|
|
||
| def test_get_r_constraints(self): | ||
| """Test the get_r_constraints() function.""" |
| # for val in zmat['map'].values(): | ||
| # if isinstance(val, str) and 'X' in val: | ||
| # idx = int(val[1:]) | ||
| # if idx > max_index: | ||
| # max_index = idx | ||
| # elif isinstance(val, int): | ||
| # if val > max_index: | ||
| # max_index = val | ||
| # | ||
| # # Fill list with Real atoms, None for placeholders up to max_index |
Check notice
Code scanning / CodeQL
Commented-out code Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, to fix commented‑out code you either (1) fully reinstate it as active code if the functionality is required and currently missing, or (2) delete it entirely if it is obsolete or superseded. Given only this snippet and no indication that this function is needed, the least risky and cleanest fix is to remove the commented‑out implementation.
Concretely, in arc/species/zmat.py around lines 2480–2518, there is a large block of what was once a function body, fully commented out line by line. To avoid changing current behavior, we will not attempt to revive or modify this logic; instead, we will delete the commented lines. No new imports, methods, or definitions are required since we are only removing dead code. The edit is a straightforward deletion of the commented block starting at the # Args: line (and possibly including any surrounding # def ... if present in the full file) through the last commented dummy_index = int(map_idx[1:]) line shown.
| # for i, coord in enumerate(xyz['coords']): | ||
| # coords_list[i] = coord | ||
| # | ||
| # # Iterate through Z-matrix rows to calculate Dummy Atom positions | ||
| # for i, row in enumerate(zmat['coords']): | ||
| # map_idx = zmat['map'][i] | ||
| # | ||
| # # Check if this is a dummy atom (mapped to string 'X...') | ||
| # if isinstance(map_idx, str) and 'X' in map_idx: | ||
| # dummy_index = int(map_idx[1:]) | ||
| # | ||
| # # Retrieve definition (R, A, D) from zmat vars | ||
| # # row format: (r_var, a_var, d_var) | ||
| # r_key, a_key, d_key = row | ||
| # | ||
| # # Helper to get parent index from var name (e.g., 'R_7_6' -> parent is 6) | ||
| # # zmat parameters are usually stored as 'Type_Current_Parent...' | ||
| # def _get_parent_idx(key_): | ||
| # if key_: | ||
| # # The second number in the string is usually the parent for R, A, D | ||
| # # e.g., R_7_6 -> 6. A_7_6_4 -> 6 (first parent). | ||
| # parts = key_.split('_') | ||
| # # parts[1] is the current atom zmat-index, parts[2] is the parent zmat-index | ||
| # parent_zmat_idx = int(parts[2]) | ||
| # # We need the xyz-index (or map value) of that parent | ||
| # parent_map_idx = zmat['map'][parent_zmat_idx] | ||
| # if isinstance(parent_map_idx, str): | ||
| # return int(parent_map_idx[1:]) | ||
| # return parent_map_idx | ||
| # return None | ||
| # | ||
| # p1_idx = _get_parent_idx(r_key) | ||
| # p2_idx = _get_parent_idx(a_key) | ||
| # p3_idx = _get_parent_idx(d_key) | ||
| # | ||
| # # Get values (Dummy definitions are assumed constant/rigid from input zmat) | ||
| # r_val = zmat['vars'][r_key] | ||
| # a_val = zmat['vars'][a_key] if a_key else 90.0 | ||
| # d_val = zmat['vars'][d_key] if d_key else 0.0 | ||
| # | ||
| # # Calculate Cartesian Position | ||
| # if p1_idx is not None and coords_list[p1_idx] is not None: | ||
| # p1 = coords_list[p1_idx] |
Check notice
Code scanning / CodeQL
Commented-out code Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the problem, the commented-out code should be removed rather than left in the file. This eliminates dead, untested logic and leaves only the active implementation in the module. If this logic is needed in the future, it can be recovered from version control.
Concretely, in arc/species/zmat.py, delete the entire commented block starting around line 2491 (# 'map': zmat['map'],) down to line 2584 (# zmat_out['vars'] = new_vars / # return zmat_out). No replacement functionality is required because the code is already entirely disabled and not part of the current behavior. No new imports, methods, or definitions are needed; we are only cleaning up unused commented code.
| # for key, val in zmat['vars'].items(): | ||
| # # Identify if this variable *defines* a dummy atom (starts with 'RX', 'AX', 'DX') | ||
| # # If so, keep original value (rigid dummy assumption). | ||
| # # Otherwise, calculate it (real atom or real atom referencing dummy). | ||
| # if key.startswith('RX') or key.startswith('AX') or key.startswith('DX'): | ||
| # new_vars[key] = val | ||
| # else: | ||
| # # Standard calculation | ||
| # # We need to get atom indices. | ||
| # indices = get_atom_indices_from_zmat_parameter(key)[0] | ||
| # | ||
| # # Map Z-mat indices to XYZ indices (handling 'X' strings) | ||
| # mapped_indices = [] | ||
| # for index in indices: | ||
| # m_val = zmat['map'][index] | ||
| # if isinstance(m_val, str) and 'X' in m_val: | ||
| # mapped_indices.append(int(m_val[1:])) | ||
| # else: | ||
| # mapped_indices.append(m_val) | ||
| # | ||
| # new_vars[key] = calculate_param(coords=coords_list, atoms=mapped_indices) |
Check notice
Code scanning / CodeQL
Commented-out code Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the problem, we should eliminate the commented-out code block rather than leaving it in place. This removes the distraction and ensures the file only contains executable code and meaningful comments. Since we must avoid changing existing functionality, we should not try to “revive” this logic; instead, we delete it, leaving the surrounding working code untouched.
Concretely, in arc/species/zmat.py, remove the entire commented-out block starting at line 2536 (# return parent_map_idx) down through line 2584 (# return zmat_out). No new imports, functions, or variables are needed, and no other regions of the file should be modified.
| @@ -2533,52 +2533,4 @@ | ||
| # parent_map_idx = zmat['map'][parent_zmat_idx] | ||
| # if isinstance(parent_map_idx, str): | ||
| # return int(parent_map_idx[1:]) | ||
| # return parent_map_idx | ||
| # return None | ||
| # | ||
| # p1_idx = _get_parent_idx(r_key) | ||
| # p2_idx = _get_parent_idx(a_key) | ||
| # p3_idx = _get_parent_idx(d_key) | ||
| # | ||
| # # Get values (Dummy definitions are assumed constant/rigid from input zmat) | ||
| # r_val = zmat['vars'][r_key] | ||
| # a_val = zmat['vars'][a_key] if a_key else 90.0 | ||
| # d_val = zmat['vars'][d_key] if d_key else 0.0 | ||
| # | ||
| # # Calculate Cartesian Position | ||
| # if p1_idx is not None and coords_list[p1_idx] is not None: | ||
| # p1 = coords_list[p1_idx] | ||
| # p2 = coords_list[p2_idx] if p2_idx is not None else None | ||
| # p3 = coords_list[p3_idx] if p3_idx is not None else None | ||
| # | ||
| # dummy_pos = get_position(r=r_val, theta=a_val, phi=d_val, p1=p1, p2=p2, p3=p3) | ||
| # coords_list[dummy_index] = dummy_pos | ||
| # | ||
| # # Now calculate new variables using the augmented coordinates list | ||
| # # Re-package coordinates into a dict structure expected by calculate_param logic if needed, | ||
| # # or just pass the list if calculate_param accepts it (it accepts lists/tuples). | ||
| # new_vars = dict() | ||
| # for key, val in zmat['vars'].items(): | ||
| # # Identify if this variable *defines* a dummy atom (starts with 'RX', 'AX', 'DX') | ||
| # # If so, keep original value (rigid dummy assumption). | ||
| # # Otherwise, calculate it (real atom or real atom referencing dummy). | ||
| # if key.startswith('RX') or key.startswith('AX') or key.startswith('DX'): | ||
| # new_vars[key] = val | ||
| # else: | ||
| # # Standard calculation | ||
| # # We need to get atom indices. | ||
| # indices = get_atom_indices_from_zmat_parameter(key)[0] | ||
| # | ||
| # # Map Z-mat indices to XYZ indices (handling 'X' strings) | ||
| # mapped_indices = [] | ||
| # for index in indices: | ||
| # m_val = zmat['map'][index] | ||
| # if isinstance(m_val, str) and 'X' in m_val: | ||
| # mapped_indices.append(int(m_val[1:])) | ||
| # else: | ||
| # mapped_indices.append(m_val) | ||
| # | ||
| # new_vars[key] = calculate_param(coords=coords_list, atoms=mapped_indices) | ||
| # | ||
| # zmat_out['vars'] = new_vars | ||
| # return zmat_out | ||
|
|
Added a method for generating TS structures from atom mapped reactants and products.
The method works for unimolecular reactions, and at present is only implemented for isomerization reactions.