-
Notifications
You must be signed in to change notification settings - Fork 150
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
raman activities with adjustable wavelength and T #967
Conversation
Signed-off-by: benbaed <[email protected]>
Signed-off-by: albert <[email protected]>
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.
Inclusion of "--hess
" and "--ptb
" calls directly into --raman
would make sense. Other than that, this looks good to me.
@@ -1778,6 +1778,18 @@ subroutine parseArguments(env, args, inputFile, paramFile, lgrad, & | |||
case ('--alpha') | |||
call set_elprop('alpha') | |||
|
|||
case ('--raman') | |||
call set_elprop('raman') |
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.
In the current state, only the electrical property is set to alpha
(via redirect raman
), but no Hessian calculation is conducted. I.e., no frequencies (and intensities) will be available.
One could add this easily via a call similar to: call set_runtyp('hess')
.
@@ -1778,6 +1778,18 @@ subroutine parseArguments(env, args, inputFile, paramFile, lgrad, & | |||
case ('--alpha') | |||
call set_elprop('alpha') | |||
|
|||
case ('--raman') | |||
call set_elprop('raman') |
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.
Similar to the comment above, a switch to turn on PTB should be added:
call set_exttyp('ptb')
if (.not. get_xtb_feature('tblite')) then
call ptb_feature_not_implemented(env)
return
endif
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.
The intended principle behind --raman
is in my opinion the combination of the flags --ptb --hess --alpha
together with Raman-specific options like temperature and frequency of incident light.
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.
(The automatic redirect to --ptb
follows from the fact that PTB is currently the only implemented method capable of calculating polarizabilities.)
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.
Good idea, but I'd still preserve --ptb
as a requirement in case of the Raman implementation with any other tight-binding method.
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.
Also fine for me. However, it should be checked whether an error is produced if a call like --raman --gfn 2
or similar is used. (Something like: "Raman not implemented with this method".)
Signed-off-by: albert <[email protected]>
Looks good to me now! |
* raman activities with adjustable wavelength and T Signed-off-by: benbaed <[email protected]> * add raman CLI opts in man page Signed-off-by: albert <[email protected]> * Update set_module.f90 * auto hess + small raman check Signed-off-by: albert <[email protected]> --------- Signed-off-by: benbaed <[email protected]> Signed-off-by: albert <[email protected]> Co-authored-by: albert <[email protected]> Co-authored-by: Marcel Mueller <[email protected]> Signed-off-by: Johannes Gorges <[email protected]>
add
ptbsetup
variablesraman_temp
andraman_lambda