-
Notifications
You must be signed in to change notification settings - Fork 15
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
Features/warning long calc #840
Features/warning long calc #840
Conversation
The check and the message were already there, but not triggered/shown.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #840 +/- ##
==========================================
+ Coverage 68.07% 68.32% +0.24%
==========================================
Files 50 50
Lines 4561 4568 +7
==========================================
+ Hits 3105 3121 +16
+ Misses 1456 1447 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
- not on localhost but big system and CPUs<4 - same as above, but on localhost - on localhost and more than 1 CPUs (we may not have enough slots)
Hi @AndresOrtegaGuerrero and @superstar54 , the PR is ready for review. In the submission step, I added (completed what I already found there) checks for: number of sites (lower than 10 is safe), cell volume (lower then 1000 A^3 is safe) and number of cpus (lower than 4 is not safe for big systems - see previous checks and Figure 1 - higher than 1 in localhost ask for checks, see Figure 2) |
you have some issues in the tests, @mikibonacci is it possible to create a test , to check this logic ? |
Hi @AndresOrtegaGuerrero, yeah will put some test, should be possible |
What about for the structure , |
Hi @AndresOrtegaGuerrero , I updated the descriptions. For the tests: I added the testing for the localhost part, not for the non-localhost (I am not sure how to do it, as we should load an external code...). Let me know if you have some ideas on how to do it, or if we can go like this. thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you Miki
This fixes #816.