Make twist attribute taking precedence over twistnum. #3799
Conversation
|
Having
|
| return twist_num; | ||
| }; | ||
|
|
||
| if (twist_inp[0] > -10 || twist_inp[1] > -10 || twist_inp[2] > -10) |
There was a problem hiding this comment.
Good catch. I am not for arbitrary constants. We should use clearer logic.
(I also think we should avoid the arbitrary sized buffers elsewhere in this code. It is not a good code pattern and one we should get away from.)
There was a problem hiding this comment.
The object which stores the input value needs to be initialized. So currently twistnum_inp and twist_inp are both set to -10.
I assume twistnum [-1, num_of_supertwist), twist [-1,1] as normal input.
Once the input parsing is done, value >-10 indicates input was given and actions need to be taken.
I think this should be good enough. This is different from an arbitrary grid number which gonna affect the accuracy.
There was a problem hiding this comment.
Just added comments in source and commit log.
There was a problem hiding this comment.
I have replaced -10 with constexpr variables.
The object which stores the input value needs to be initialized. So currently twistnum_inp and twist_inp are both set to -10. I assume twistnum [-1, num_of_supertwist), twist [-1,1] as normal input. Once the input parsing is done, value >-10 indicates input was given and actions need to be taken.
|
Test this please |
|
Test this please |
|
Test this please |
Proposed changes
Once twist is detected, twistnum is ignored with a warning if found.
If there is only twistnum, take it and print a warning about its ambiguity.
If nothing found, set Gamma.
before the change from this PR, when both twistnum and twist are not provided, the default is twistnum=0.
This is a risky default as twistnum is risky. I made the choice to use Gamma point as the default to reduce input burden. @lshulen suggested just put an error requiring either twist or twistnum input. Let me know your choice.
Note I'm not changing tests in this PR to avoid complicating the review. They will be updated in a separate PR and it does require #3797.
What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
epyc-server
Checklist