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

patch to allow scientific notation dielectric function, with possibil… #1836

Merged
merged 4 commits into from
Apr 28, 2020

Conversation

computron
Copy link
Member

Summary

Some versions of VASP (notably, run with MedeA) have a different format to dielectric function tensor output in the OUTCAR file. Notably, the format:

  • uses scientific notation
  • may not have any spaces or delimiter character in between adjacent values on a line

For example, the OUTCAR may contain a section that looks like:

  frequency dependent IMAGINARY DIELECTRIC FUNCTION (independent particle, no local field effects) density-density
     E(ev)      X         Y         Z        XY        YZ        ZX
  --------------------------------------------------------------------------------------------------------------
 0.00000E+00 0.00000E+00 0.00000E+00 0.00000E+00 0.00000E+00 0.00000E+00 0.00000E+00
 0.62963E+00 0.15467E+02 0.15467E+02 0.15467E+02-0.30654E-16-0.91612E-16 0.52388E-16

Note the use of scientific notation throughout, as well as the lack of delimiter between some of the values in the second line.

…ity of no spaces in between the different values due to overflow.
@shyuep
Copy link
Member

shyuep commented Apr 25, 2020

Can we have a unitest of this. No need for a full OUTCAR. Just testing the regex alone would be sufficient.

@computron
Copy link
Member Author

The regex is within the read_freq_dielectric() function in the OUTCAR class - which means I can't run the regex without first parsing an entire OUTCAR. As far as I can tell, my options are:

  1. Moving the regex pattern to somewhere universally accessible, e.g., out of read_freq_dielectric and at the top level of the outputs.py module, and then writing a test on that pattern (since now I can access the regex pattern without calling read_freq_dielectric(). This is fine but the code would look a little weirder

  2. Move the function to parse the scientific notation rows to its own static method in Outcar, and then test that static function. Again possible, but again the code would look weirder since now there is a whole other function to do essentially a one-line check.

@shyuep
Copy link
Member

shyuep commented Apr 25, 2020

  1. is fine. But I would think splitting a sequence of scientific notation numbers is useful in more than one place. So 2 would be useful. I really don't think this is the only place with this particular problem.

@shyamd
Copy link
Contributor

shyamd commented Apr 27, 2020

If you make a regex that can handle both regular notation and scientific, we can just slowly patch that into the various functions. Note that python float interprets scientific notation just fine.

@computron
Copy link
Member Author

Ok I separated the function out and added a unit test

Note that I think it would be fairly straightforward to try to parse both types of numbers, assuming that if there is no exponent that there is at least a space between values (just change the exponent in the regex to be either the exponent or a set of whitespaces). But, I didn't attempt that here.

@shyuep shyuep merged commit d73e343 into master Apr 28, 2020
@shyuep
Copy link
Member

shyuep commented Apr 28, 2020

thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants