Skip to content

Commit

Permalink
Merge pull request #464 from ReactionMechanismGenerator/minor_style
Browse files Browse the repository at this point in the history
A collection of minor style modification commits
  • Loading branch information
xiaoruiDong committed Sep 22, 2021
2 parents 4e0dc47 + 2c37ef4 commit 7f9187e
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 105 deletions.
26 changes: 11 additions & 15 deletions arc/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@

logger = logging.getLogger('arc')

# absolute path to the ARC folder
# Absolute path to the ARC folder.
ARC_PATH = os.path.abspath(os.path.dirname(os.path.dirname(__file__)))
# absolute path to RMG-Py folder
# Absolute path to RMG-Py folder.
RMG_PATH = os.path.abspath(os.path.dirname(os.path.dirname(rmgpy.__file__)))
# absolute path to RMG-database folder
# Absolute path to RMG-database folder.
RMG_DATABASE_PATH = os.path.abspath(os.path.dirname(rmgpy.settings['database.directory']))

VERSION = '1.1.0'
Expand Down Expand Up @@ -387,7 +387,6 @@ def save_yaml_file(path: str,
"""
if not isinstance(path, str):
raise InputError(f'path must be a string, got {path} which is a {type(path)}')
logger.debug('Creating a restart file...')
yaml_str = to_yaml(py_content=content)
if '/' in path and os.path.dirname(path) and not os.path.exists(os.path.dirname(path)):
os.makedirs(os.path.dirname(path))
Expand Down Expand Up @@ -549,17 +548,14 @@ def colliding_atoms(xyz: dict,
xyz (dict): The Cartesian coordinates.
threshold (float, optional): The collision threshold to use.
Returns: bool
``True`` if they are colliding, ``False`` otherwise.
Returns:
bool: ``True`` if they are colliding, ``False`` otherwise.
"""
if len(xyz['symbols']) == 1:
# monoatomic
return False

geometry = np.array([np.array(coord, np.float64) * 1.8897259886 for coord in xyz['coords']]) # convert A to Bohr
geometry = np.array([np.array(coord, np.float64) * 1.8897259886 for coord in xyz['coords']]) # Convert A to Bohr.
qcel_out = qcel.molutil.guess_connectivity(symbols=xyz['symbols'], geometry=geometry, threshold=threshold)
logger.debug(qcel_out)

return bool(len(qcel_out))


Expand Down Expand Up @@ -830,8 +826,8 @@ def almost_equal_coords(xyz1: dict,
return True


def almost_equal_coords_lists(xyz1: dict,
xyz2: dict,
def almost_equal_coords_lists(xyz1: Union[List[dict], dict],
xyz2: Union[List[dict], dict],
rtol: float = 1e-05,
atol: float = 1e-08,
) -> bool:
Expand All @@ -840,8 +836,8 @@ def almost_equal_coords_lists(xyz1: dict,
Useful for comparing xyz's in unit tests.
Args:
xyz1 (list, dict): Either a dict-format xyz, or a list of them.
xyz2 (list, dict): Either a dict-format xyz, or a list of them.
xyz1 (Union[List[dict], dict]): Either a dict-format xyz, or a list of them.
xyz2 (Union[List[dict], dict]): Either a dict-format xyz, or a list of them.
rtol (float, optional): The relative tolerance parameter.
atol (float, optional): The absolute tolerance parameter.
Expand Down Expand Up @@ -1217,7 +1213,7 @@ def torsions_to_scans(descriptor: Optional[List[List[int]]],
raise TypeError(f'Expected a list, got {descriptor} which is a {type(descriptor)}')
if not isinstance(descriptor[0], (list, tuple)):
descriptor = [descriptor]
direction = direction if direction == 1 else -1 # anything other than 1 is translated to -1
direction = direction if direction == 1 else -1 # Anything other than 1 is translated to -1.
new_descriptor = [convert_list_index_0_to_1(entry, direction) for entry in descriptor]
if any(any(item < 0 for item in entry) for entry in new_descriptor):
raise ValueError(f'Got an illegal value when converting:\n{descriptor}\ninto:\n{new_descriptor}')
Expand Down
17 changes: 9 additions & 8 deletions arc/job/trsh.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,27 +477,28 @@ def trsh_negative_freq(label: str,
if len(neg_freqs_idx) == 1 and not len(neg_freqs_trshed):
# species has one negative frequency, and has not been troubleshooted for it before
logger.info(f'Species {label} has a negative frequency ({freqs[largest_neg_freq_idx]}). Perturbing its '
f'geometry using the respective vibrational displacements')
f'geometry using the respective vibrational normal mode displacement(s).')
neg_freqs_idx = [largest_neg_freq_idx] # indices of the negative frequencies to troubleshoot for
elif len(neg_freqs_idx) == 1 and any([np.allclose(freqs[0], vf, rtol=1e-04, atol=1e-02)
for vf in neg_freqs_trshed]):
# species has one negative frequency, and has been troubleshooted for it before
factor = 1 + factor_increase * (len(neg_freqs_trshed) + 1)
logger.info(f'Species {label} has a negative frequency ({freqs[largest_neg_freq_idx]}) for the '
f'{len(neg_freqs_trshed)} time. Perturbing its geometry using the respective vibrational '
f'displacements, this time using a larger factor (x {factor})')
f'normal mode displacement(s), this time using a larger factor (x {factor})')
neg_freqs_idx = [largest_neg_freq_idx] # indices of the negative frequencies to troubleshoot for
elif len(neg_freqs_idx) > 1 and not any([np.allclose(freqs[0], vf, rtol=1e-04, atol=1e-02)
for vf in neg_freqs_trshed]):
# species has more than one negative frequency, and has not been troubleshooted for it before
logger.info(f'Species {label} has {len(neg_freqs_idx)} negative frequencies. Perturbing its geometry using the vibrational '
f'displacements of its largest negative frequency, {freqs[largest_neg_freq_idx]}')
logger.info(f'Species {label} has {len(neg_freqs_idx)} negative frequencies. Perturbing its geometry using '
f'the vibrational normal mode displacement(s) of its largest negative frequency, '
f'{freqs[largest_neg_freq_idx]}')
neg_freqs_idx = [largest_neg_freq_idx] # indices of the negative frequencies to troubleshoot for
elif len(neg_freqs_idx) > 1 and any([np.allclose(freqs[0], vf, rtol=1e-04, atol=1e-02)
for vf in neg_freqs_trshed]):
# species has more than one negative frequency, and has been troubleshooted for it before
logger.info(f'Species {label} has {len(neg_freqs_idx)} negative frequencies. Perturbing its geometry '
f'using the vibrational displacements of ALL negative frequencies')
f'using the vibrational normal mode displacement(s) of ALL negative frequencies')
# convert a numpy array to a list, imprtant for saving the neg_freqs_trshed species attribute in the restart
freqs_list = freqs.tolist()
current_neg_freqs_trshed = [round(freqs_list[i], 2) for i in neg_freqs_idx] # record trshed negative freqs
Expand Down Expand Up @@ -529,7 +530,7 @@ def trsh_scan_job(label: str,
Args:
label (str): The species label.
scan_res (int or float): The scan resolution in degrees.
scan_res (int, float): The scan resolution in degrees.
scan (list): The four atom indices representing the torsion to be troubleshooted.
scan_list (list): Entries are the four-atom scan lists (1-indexed) of all torsions
(without duplicate pivots) in this species.
Expand Down Expand Up @@ -1344,7 +1345,7 @@ def scan_quality_check(label: str,

# 1.3 Check consistency
if 0 in changed_ic_dict.keys() and len(changed_ic_dict) == 1:
# Smooth scan with different initial and final conformer
# A smooth scan with different initial and final conformer.
invalidate = True
invalidation_reason = 'Inconsistent initial and final conformers'
message = f'Rotor scan of {label} between pivots {pivots} has inconsistent initial ' \
Expand All @@ -1355,7 +1356,7 @@ def scan_quality_check(label: str,
for ic_label in changed_ic_dict[0]]}
return invalidate, invalidation_reason, message, actions
elif len(changed_ic_dict) > 0:
# Not smooth scan
# Not a smooth scan.
invalidate = True
invalidation_reason = 'Significant difference observed between consecutive conformers'
message = f'Rotor scan of {label} between pivots {pivots} is inconsistent between ' \
Expand Down
18 changes: 9 additions & 9 deletions arc/job/trshTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,16 +550,16 @@ def test_scan_quality_check(self):
'scan_res': 4.0,
'used_methods': None,
'log_file': log_file,
}
}
invalidate, invalidation_reason, message, actions = trsh.scan_quality_check(**case1)
self.assertTrue(invalidate)
self.assertEqual(
invalidation_reason, 'Significant difference observed between consecutive conformers')
expect_message = 'Rotor scan of CH2OOH between pivots [1, 2] is inconsistent between ' \
'two consecutive conformers.\nInconsistent consecutive conformers and ' \
'problematic internal coordinates:\nconformer # 63 / # 64 D3, D2' \
'\nconformer # 80 / # 81 D3, D2\nARC will attempt to troubleshoot' \
' this rotor scan.'
'\nconformer # 80 / # 81 D3, D2\n' \
'ARC will attempt to troubleshoot this rotor scan.'
self.assertEqual(message, expect_message)
self.assertEqual(len(actions.keys()), 1)
self.assertIn('freeze', actions)
Expand All @@ -574,7 +574,7 @@ def test_scan_quality_check(self):
'scan_res': 8.0,
'used_methods': None,
'log_file': log_file,
}
}
invalidate, invalidation_reason, message, actions = trsh.scan_quality_check(**case2)
self.assertTrue(invalidate)
self.assertEqual(
Expand Down Expand Up @@ -605,11 +605,11 @@ def test_scan_quality_check(self):
def test_trsh_scan_job(self):
"""Test troubleshooting problematic 1D rotor scan"""
case = {'label': 'CH2OOH',
'scan_res': 4.0,
'scan': [4, 1, 2, 3],
'scan_list': [[4, 1, 2, 3], [1, 2, 3, 6]],
'methods': {'freeze': [[5, 1, 2, 3], [2, 1, 4, 5]]},
'log_file': os.path.join(ARC_PATH, 'arc', 'testing', 'rotor_scans', 'CH2OOH.out'),
'scan_res': 4.0,
'scan': [4, 1, 2, 3],
'scan_list': [[4, 1, 2, 3], [1, 2, 3, 6]],
'methods': {'freeze': [[5, 1, 2, 3], [2, 1, 4, 5]]},
'log_file': os.path.join(ARC_PATH, 'arc', 'testing', 'rotor_scans', 'CH2OOH.out'),
}
scan_trsh, scan_res = trsh.trsh_scan_job(**case)
self.assertEqual(scan_trsh, 'D 5 4 1 2 F\nD 1 2 3 6 F\nB 2 3 F\n')
Expand Down
14 changes: 7 additions & 7 deletions arc/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,16 +391,16 @@ def __init__(self,
logger.info(f'Using the following adaptive levels of theory:\n{self.adaptive_levels}')
self.ess_settings = check_ess_settings(ess_settings or global_ess_settings)
if not self.ess_settings:
# use the "radar" feature if ess_settings are still unavailable
# Use the "radar" feature if ess_settings are still unavailable.
self.determine_ess_settings()

# Determine if fine-only behavior is requested before determining chemistry for job types
# Determine if fine-only behavior is requested before determining chemistry for job types.
self.fine_only = False
if self.job_types['fine'] and not self.job_types['opt']:
self.fine_only = True
self.job_types['opt'] = True # Run the optimizations, self.fine_only will make sure that they are fine
self.job_types['opt'] = True # Run the optimizations, self.fine_only will make sure that they are fine.

self.set_levels_of_theory() # all level of theories should be Level types after this call
self.set_levels_of_theory() # All level of theories should be Level types after this call.
if self.thermo_adapter == 'Arkane':
self.check_arkane_level_of_theory()

Expand Down Expand Up @@ -817,21 +817,21 @@ def check_project_name(self):
for char in self.project:
if char not in valid_chars:
raise InputError(f'A project name (used to naming folders) must contain only valid characters. '
f'Got {char} in {self.project}.')
f'Got "{char}" in {self.project}.')

def check_freq_scaling_factor(self):
"""
Check that the harmonic frequencies scaling factor is known,
otherwise, and if ``calc_freq_factor`` is set to ``True``, spawn a calculation for it using Truhlar's method.
"""
if self.freq_scale_factor is None:
# the user did not specify a scaling factor, see if Arkane has it
# The user did not specify a scaling factor, see if Arkane has it.
freq_level = self.composite_method if self.composite_method is not None \
else self.freq_level if self.freq_level is not None else None
if freq_level is not None:
arkane_freq_lot = freq_level.to_arkane_level_of_theory(variant='freq')
if arkane_freq_lot is not None:
# Arkane has this harmonic frequencies scaling factor
# Arkane has this harmonic frequencies scaling factor.
self.freq_scale_factor = assign_frequency_scale_factor(level_of_theory=arkane_freq_lot)
else:
logger.info(f'Could not determine the harmonic frequencies scaling factor for '
Expand Down
4 changes: 2 additions & 2 deletions arc/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def parse_t1(path: str) -> Optional[float]:
The T1 parameter.
"""
if not os.path.isfile(path):
raise InputError('Could not find file {0}'.format(path))
raise InputError(f'Could not find file {path}')
log = ess_factory(fullpath=path, check_for_errors=False)
try:
t1 = log.get_T1_diagnostic()
Expand Down Expand Up @@ -264,7 +264,7 @@ def parse_zpe(path: str) -> Optional[float]:
The calculated zero point energy in kJ/mol.
"""
if not os.path.isfile(path):
raise InputError('Could not find file {0}'.format(path))
raise InputError(f'Could not find file {path}')
log = ess_factory(fullpath=path, check_for_errors=False)
try:
zpe = log.load_zero_point_energy() * 0.001 # convert to kJ/mol
Expand Down
4 changes: 2 additions & 2 deletions arc/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ def process_arc_project(thermo_adapter: str,
if compute_rates:
for reaction in reactions:
species_converged = True
considered_labels = list() # species labels considered in this reaction
considered_labels = list() # Species labels considered in this reaction.
if output_dict[reaction.ts_label]['convergence']:
for species in reaction.r_species + reaction.p_species:
if species.label in considered_labels:
# consider cases where the same species appears in a reaction both as a reactant
# Consider cases where the same species appears in a reaction both as a reactant
# and as a product (e.g., H2O that catalyzes a reaction).
continue
considered_labels.append(species.label)
Expand Down
10 changes: 6 additions & 4 deletions arc/reaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,11 @@ def rmg_reaction_from_str(self, reaction_string: str):
def rmg_reaction_from_arc_species(self):
"""
A helper function for generating the RMG Reaction object from ARCSpecies
Used for determining the family
Used for determining the family.
"""
if self.rmg_reaction is None and len(self.r_species) and len(self.p_species) and \
all([arc_spc.mol is not None for arc_spc in self.r_species + self.p_species]):
reactants, products = self.get_reactants_and_products(arc=False) # Return RMG Species.
reactants, products = self.get_reactants_and_products(arc=False) # Returns RMG Species.
self.rmg_reaction = Reaction(reactants=reactants, products=products)

def arc_species_from_rmg_reaction(self):
Expand Down Expand Up @@ -743,7 +743,7 @@ def get_reactants_and_products(self,
) -> Tuple[List[Union[ARCSpecies, Species]], List[Union[ARCSpecies, Species]]]:
"""
Get a list of reactant and product species including duplicate species, if any.
The species could either be ARCSpecies or RMGSpecies.
The species could either be ``ARCSpecies`` or ``RMGSpecies`` object instance.
Args:
arc (bool, optional): Whether to return the species as ARCSpecies (``True``) or as RMG Species (``False``).
Expand Down Expand Up @@ -838,9 +838,11 @@ def get_products_xyz(self, return_format='str') -> Union[dict, str]:
"""
Get a combined string/dict representation of the cartesian coordinates of all product species.
The resulting coordinates are ordered as the reactants using an atom map.
Args:
return_format (str): Either ``'dict'`` to return a dict format or ``'str'`` to return a string format.
Default: ``'str'``.
Default: ``'str'``.
Returns: Union[dict, str]
The combined cartesian coordinates
Todo:
Expand Down

0 comments on commit 7f9187e

Please sign in to comment.