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

Scan l103 #687

Merged
merged 4 commits into from
Oct 5, 2023
Merged

Scan l103 #687

merged 4 commits into from
Oct 5, 2023

Conversation

alongd
Copy link
Member

@alongd alongd commented Jul 28, 2023

Invalidate scan jobs that failed with internal coordinate error (L103)
A test was added

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #687 (611df4e) into main (6e84f83) will increase coverage by 0.19%.
The diff coverage is 88.00%.

@@            Coverage Diff             @@
##             main     #687      +/-   ##
==========================================
+ Coverage   73.28%   73.48%   +0.19%     
==========================================
  Files          99       99              
  Lines       26558    26576      +18     
  Branches     5538     5539       +1     
==========================================
+ Hits        19464    19530      +66     
+ Misses       5746     5681      -65     
- Partials     1348     1365      +17     
Flag Coverage Δ
unittests 73.48% <88.00%> (+0.19%) ⬆️

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

Files Coverage Δ
arc/job/adapters/gaussian.py 67.34% <100.00%> (ø)
arc/scheduler_test.py 100.00% <100.00%> (ø)
arc/scheduler.py 23.15% <57.14%> (+1.36%) ⬆️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

Choose a reason for hiding this comment

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

When I run the whole test file locally, I have the following error that I am not familiar with. Not sure if it is crucial.

======================================================================
ERROR: test_check_rxn_e0_by_spc (__main__.TestScheduler)
Test the check_rxn_e0_by_spc() method.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/user/Desktop/New/ARC/arc/scheduler_test.py", line 693, in test_check_rxn_e0_by_spc
    'arc_project_for_testing_delete_after_usage6'),
  File "/Users/user/Desktop/New/ARC/arc/job/factory.py", line 209, in job_factory
    xyz=xyz,
  File "/Users/user/Desktop/New/ARC/arc/job/adapters/gaussian.py", line 195, in __init__
    xyz=xyz,
  File "/Users/user/Desktop/New/ARC/arc/job/adapters/common.py", line 209, in _initialize_adapter
    if obj.execution_type != 'incore' and obj.job_adapter in obj.ess_settings.keys():
AttributeError: 'NoneType' object has no attribute 'keys'

@@ -2653,25 +2653,24 @@ def check_scan_job(self,
label (str): The species label.
job (JobAdapter): The rotor scan job object.
"""
# If the job has not converged, troubleshoot ESS.
# Besides, according to the experience, 'Internal coordinate error' cannot be handled by
Copy link
Member

Choose a reason for hiding this comment

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

Would you like to keep the comment here? As a new user here, I don't know 'Internal coordinate error' cannot be handled by troubleshoot_ess() for scan jobs. With that in mind, I understand why you modify the code into the updated format.

arc/scheduler.py Outdated
@@ -2727,7 +2728,7 @@ def check_scan_job(self,

if invalidate:
self.species_dict[label].rotors_dict[job.rotor_index]['success'] = False
self.species_dict[label].rotors_dict[job.rotor_index]['invalidation_reason'] = invalidation_reason
# self.species_dict[label].rotors_dict[job.rotor_index]['invalidation_reason'] = invalidation_reason
Copy link
Collaborator

Choose a reason for hiding this comment

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

@alongd Do you want to remove this comment if it is not required?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure why I left this commented out, I'll un-comment it

@kfir4444 kfir4444 merged commit 067e76c into main Oct 5, 2023
7 checks passed
@kfir4444 kfir4444 deleted the scan_l103 branch October 5, 2023 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants