Improvement/process labels #836
Conversation
Following the guidelines in aiidalab#811
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #836 +/- ##
==========================================
+ Coverage 67.92% 67.96% +0.04%
==========================================
Files 49 49
Lines 4383 4386 +3
==========================================
+ Hits 2977 2981 +4
+ Misses 1406 1405 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
What do you think ? |
| else: | ||
| relax_info = "structure is not relaxed" | ||
| relax_info = ( | ||
| "relax: atoms+cell" if "cell" in relax_type else "relax: atoms only" |
There was a problem hiding this comment.
I would map relax as
Relax: Atoms/Cell
Relax: Atoms
Relax: None
There was a problem hiding this comment.
I would keep lowercase everything (so also the properties do not need to be uppercased) and keep explicit the protocol word, as who is not expert in AiiDA may not immediately understand that we are indicating the protocol (in particular in the moderate case, I would say).
I like the new format of your comment and the new relax mapping, but I would keep atoms+cell, as atoms/cell may sound misleading (not immediately clear that is a vc-relax). Or maybe atoms&cell? But I don't have a strong preference.
What do you think, @AndresOrtegaGuerrero ? Am I too paranoic? 😃
There was a problem hiding this comment.
Yeah the lowercase is not a problem. And I see your point of the protocol. Yes you can keep the "+" does the same effect. No you aren't paranoic xD
|
@mikibonacci
It could be nice that for the label for the job, it already uses the label that the user is giving to the structure |
The two will coincide if the user does not modify the default label. Also, if by mistake the user provides an empty label, automatically the job label will use the structure formula.
…onacci/aiidalab-qe into improvement/process_labels
|
Hi @AndresOrtegaGuerrero , I added the structure label as default label (then, if empty label is provided, we still use the formula). I also adapted to the format you suggested |
| assert ( | ||
| submit_step.process_label.value | ||
| == "Si2 [relax: atoms+cell, moderate protocol] → bands, pdos" | ||
| ) |
There was a problem hiding this comment.
Could we add an extra test , where the structure has a label ?
|
Hi @AndresOrtegaGuerrero , I added (in the same test) the check if we provide a label != formula and an empty label (we fallback into the formula) |
|
I will just see if I can capitalize all the PDOS in the app, then I will merge |
This fixes #811.
Still missing, capitalize PDOS everywhere in the GUI. Coming soon (but not really relevant for the PR review)