Report energy in `conformers_before_optimization.txt` #743

JintaoWu98 · 2024-05-21T05:29:39Z

The previous conformers_before_optimization.txt doesn't include energy information for the conformer before the optimization, so the updated one will include this.

arc/plotter.py

codecov · 2024-05-21T06:22:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.81%. Comparing base (3631196) to head (9591699).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #743      +/-   ##
==========================================
+ Coverage   73.80%   73.81%   +0.01%     
==========================================
  Files          99       99              
  Lines       27352    27356       +4     
  Branches     5718     5719       +1     
==========================================
+ Hits        20187    20193       +6     
+ Misses       5738     5737       -1     
+ Partials     1427     1426       -1

Flag	Coverage Δ
unittests	`73.81% <100.00%> (+0.01%)`	⬆️

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

Thanks! I added some minor comments

arc/plotter.py

alongd · 2024-05-21T16:53:36Z

arc/plotter.py

    with open(conf_path, 'w') as f:
        content = ''
        if optimized:
            level_of_theory = level_of_theory.simple() if isinstance(level_of_theory, Level) else level_of_theory
            content += f'Conformers for {label}, optimized at the {level_of_theory} level:\n\n'
+        if before_optimized:
+            level_of_theory = level_of_theory.simple() if isinstance(level_of_theory, Level) else level_of_theory
+            content += f'Conformers for {label}, generated from RDKit:\n\n'


You can pass the string level of theory into this function (e.g., MMFF94s or UFF) and print it here instead of writing RDKit

alongd · 2024-05-21T16:54:19Z

arc/scheduler.py

@@ -1845,6 +1845,8 @@ def process_conformers(self, label):
                                     multiplicity=self.species_dict[label].multiplicity,
                                     charge=self.species_dict[label].charge,
                                     is_ts=False,
+                                     energies=self.species_dict[label].conformer_energies,
+                                     before_optimized=True,


Please rename to "before_optimization"

codecov-commenter · 2024-05-24T01:13:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.09%. Comparing base (5a82b3c) to head (e9de000).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #743   +/-   ##
=======================================
  Coverage   74.08%   74.09%           
=======================================
  Files         101      101           
  Lines       28007    28005    -2     
  Branches     5860     5860           
=======================================
+ Hits        20750    20751    +1     
+ Misses       5787     5786    -1     
+ Partials     1470     1468    -2

Flag	Coverage Δ
unittests	`74.09% <100.00%> (+<0.01%)`	⬆️

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

Thanks, please see some comments

arc/plotter.py

alongd · 2024-05-25T05:02:52Z

arc/plotter.py

@@ -923,18 +924,21 @@ def save_conformers_file(project_directory: str,
    geo_dir = os.path.join(project_directory, 'output', spc_dir, label, 'geometry', 'conformers')
    if not os.path.exists(geo_dir):
        os.makedirs(geo_dir)
-    if energies is not None and any(e is not None for e in energies):
+    if before_optimization or energies is None or not any(e is not None for e in energies):


We should make the conditions much simpler and differentiate the case simply using the before_optimization arg instead of looking at the energies. something like:
Also, now we don't need to set optimized, we can just use before_optimization.
Can you go over the logic and make this function simpler and robust?

alongd · 2024-05-25T05:07:18Z

arc/plotter.py

@@ -900,6 +900,7 @@ def save_conformers_file(project_directory: str,
                         ts_methods: Optional[List[str]] = None,
                         im_freqs: Optional[List[List[float]]] = None,
                         log_content: bool = False,
+                         before_optimization: Optional[bool] = None,


I think this shouldn't be Optional (cannot be None) because we're always either before or after opt.
We should make sure to send a proper value in all the calls to this functions, including tests

alongd

Thanks! Please see two minor comments

arc/scheduler.py

alongd · 2024-05-25T15:41:08Z

arc/species/species.py

@@ -1098,7 +1098,7 @@ def generate_conformers(self,
                                                      )
        if len(lowest_confs):
            self.conformers.extend([conf['xyz'] for conf in lowest_confs])
-            self.conformer_energies.extend([None] * len(lowest_confs))
+            self.conformer_energies.extend([conf['FF energy'] for conf in lowest_confs])


Can you explain what this does?

ARC/arc/scheduler.py

Lines 1842 to 1851 in 98b443c

plotter.save_conformers_file(project_directory=self.project_directory,

label=label,

xyzs=self.species_dict[label].conformers,

level_of_theory=self.conformer_level,

multiplicity=self.species_dict[label].multiplicity,

charge=self.species_dict[label].charge,

is_ts=False,

energies=self.species_dict[label].conformer_energies,

before_optimization=True,

) # before optimization

Line 1849 self.species_dict[label].conformer_energies requires attribute conformer_energies to be well defined, previous code will lead to a list of None, rather than the actual values.

Now it decides the condition based on the new var `before_optimization`

alongd

Thanks!

JintaoWu98 requested a review from alongd May 21, 2024 05:29

github-actions bot added Module: Plotter Module: Scheduler labels May 21, 2024

github-advanced-security bot found potential problems May 21, 2024

View reviewed changes

arc/plotter.py Fixed Show resolved Hide resolved

alongd reviewed May 21, 2024

View reviewed changes

JintaoWu98 force-pushed the conf_before_opt branch 2 times, most recently from 9591699 to 8470e4f Compare May 24, 2024 01:13

JintaoWu98 force-pushed the conf_before_opt branch from 8470e4f to 9106a3c Compare May 25, 2024 00:23

alongd reviewed May 25, 2024

View reviewed changes

JintaoWu98 force-pushed the conf_before_opt branch from 30d4952 to 357696d Compare May 25, 2024 09:28

github-actions bot added the Module: Species label May 25, 2024

JintaoWu98 force-pushed the conf_before_opt branch 4 times, most recently from 10cd5b8 to 98b443c Compare May 25, 2024 12:01

alongd reviewed May 25, 2024

View reviewed changes

JintaoWu98 force-pushed the conf_before_opt branch from 98b443c to e53ab9e Compare May 26, 2024 00:14

JintaoWu98 added 5 commits September 3, 2024 15:12

Define new variable before_optimization for function save_conformer

a69af51

Simplify the logic of function save_conformer

b62a5e0

Now it decides the condition based on the new var `before_optimization`

Define the attribute conformer_energies properly

f751108

Modify the logic for conformer min_e

f9a2535

Modify the tests

e9de000

JintaoWu98 force-pushed the conf_before_opt branch from e53ab9e to e9de000 Compare September 3, 2024 07:12

alongd approved these changes Sep 9, 2024

View reviewed changes

alongd merged commit fb16f4a into main Sep 9, 2024
7 checks passed

alongd deleted the conf_before_opt branch September 9, 2024 18:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report energy in `conformers_before_optimization.txt` #743

Report energy in `conformers_before_optimization.txt` #743

JintaoWu98 commented May 21, 2024

codecov bot commented May 21, 2024 •

edited

Loading

alongd left a comment

alongd May 21, 2024

alongd May 21, 2024

codecov-commenter commented May 24, 2024 •

edited by codecov bot

Loading

alongd left a comment

alongd May 25, 2024

alongd May 25, 2024

alongd left a comment

alongd May 25, 2024

JintaoWu98 May 25, 2024 •

edited

Loading

alongd left a comment

	plotter.save_conformers_file(project_directory=self.project_directory,
	label=label,
	xyzs=self.species_dict[label].conformers,
	level_of_theory=self.conformer_level,
	multiplicity=self.species_dict[label].multiplicity,
	charge=self.species_dict[label].charge,
	is_ts=False,
	energies=self.species_dict[label].conformer_energies,
	before_optimization=True,
	) # before optimization

Report energy in conformers_before_optimization.txt #743

Report energy in conformers_before_optimization.txt #743

Conversation

JintaoWu98 commented May 21, 2024

codecov bot commented May 21, 2024 • edited Loading

Codecov Report

alongd left a comment

Choose a reason for hiding this comment

alongd May 21, 2024

Choose a reason for hiding this comment

alongd May 21, 2024

Choose a reason for hiding this comment

codecov-commenter commented May 24, 2024 • edited by codecov bot Loading

Codecov Report

alongd left a comment

Choose a reason for hiding this comment

alongd May 25, 2024

Choose a reason for hiding this comment

alongd May 25, 2024

Choose a reason for hiding this comment

alongd left a comment

Choose a reason for hiding this comment

alongd May 25, 2024

Choose a reason for hiding this comment

JintaoWu98 May 25, 2024 • edited Loading

Choose a reason for hiding this comment

alongd left a comment

Choose a reason for hiding this comment

Report energy in `conformers_before_optimization.txt` #743

Report energy in `conformers_before_optimization.txt` #743

codecov bot commented May 21, 2024 •

edited

Loading

codecov-commenter commented May 24, 2024 •

edited by codecov bot

Loading

JintaoWu98 May 25, 2024 •

edited

Loading