Implement write function of upf file #12
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
==========================================
- Coverage 82.45% 80.05% -2.40%
==========================================
Files 22 23 +1
Lines 1533 1760 +227
==========================================
+ Hits 1264 1409 +145
- Misses 269 351 +82 ☔ View full report in Codecov by Sentry. |
|
Really cool, thanks, Jason! I'll look this over tomorrow; hopefully we can get it merged soon. |
| # New UPF files with schema are in XML and start with a version tag | ||
| return 2 | ||
| elseif occursin("xml version=\"1.0\"", line1) && occursin("UPF version=\"2.0.1\"", line2) |
There was a problem hiding this comment.
UPF version 2 files should start with 'UPF version="2.0.1"'. I guess EzXML automatically adds the xml version tag, but we should remove it from the UPF files before writing.
There was a problem hiding this comment.
Interesting.. I remove the xml line from the output file anyway.
However, I noticed the UPF files generated by ld1.x code from QE v.6.3MaX all include this line. In order to read those files correctly, the condition here is needed.
There was a problem hiding this comment.
Very strange... they don't even follow their own spec!
| @@ -112,13 +163,53 @@ function upf2_parse_header(doc::EzXML.Document) | |||
| return upf2_parse_header(findfirst("PP_HEADER", root(doc))) | |||
| end | |||
|
|
|||
| function upf2_dump_header(h::UpfHeader)::EzXML.Node | |||
There was a problem hiding this comment.
This could probably be done more elegantly by iterating over the fields / properties of UpfHeader because the field names are the same as the keys in the UPF file.
There was a problem hiding this comment.
We can actually do this everywhere there are lots of set_attr! calls.
There was a problem hiding this comment.
This is the most difficult decision 😄 , if there are > 10 set_attr!, sure to call use a for loop, what if there are 2, or 3?
I'll check and see later today. I think three is a fair line, since the change after are three lines.
src/file/upf2.jl
Outdated
| set_attr!(node, "norm_conserving_radius", beta.norm_conserving_radius) | ||
| set_attr!(node, "ultrasoft_cutoff_radius", beta.ultrasoft_cutoff_radius) | ||
| set_attr!(node, "label", beta.label) | ||
| beta = [beta.beta; zeros(mesh_size - length(beta.beta))] |
There was a problem hiding this comment.
I should have committed a change I was planning to make; the beta projectors should not actually be truncated to the cutoff_radius_index.
There was a problem hiding this comment.
It is fixed in c068401 (I guess the truncate was because UPF1 and UPF2 has different sizes for beta, you adopt UPF2 beta vector size to conform with UPF1. In this commit, I do the opposite.)
There was a problem hiding this comment.
No, the truncation was because the UPF spec recommends to always cut off the beta projectors at the cutoff_radius_index. However, in practice, QE does not do this. It reads all the values (zeros/whatever) and then uses the largest cutoff_radius_index for all the beta projectors from a given pseudo.
In any case, it's cleaner to just read all the data (that way, it has the same dimension as the radial mesh). Then, if you want to truncate when using the pseudo, the data is there.
unkcpz
force-pushed
the
fea/xx/io_write_of_upf_psp8
branch
2 times, most recently
from
fa5cef4
to
f0029a9
Compare
unkcpz
force-pushed
the
fea/xx/io_write_of_upf_psp8
branch
from
f0029a9
to
3e47179
Compare
|
Thanks again for the great work! You've inspired me a bit to give a crack at the I just made a draft PR #13 that I'll merge after this is done. There, I've implemented UPF <--> PSP8 conversion for non-relativistic pseudos and a writer for PSP8. This should make it possible to convert UPF files to PSP8 files (on disk) and back (the conversion process is a bit lossy, but all the important parts are retained). |
Super cool, and fast! I worked on this PR because I want to do trim on the UPF as you guys did in the ACWF paper which is a small goal, then I went to the ambition to exactly you did for the converter between upf <-> psp8 but give up on some details. Could you ping me to have a look at your PR before merging? I am looking forward to learning more details. |
|
No idea how to fix failed nightly tests. It is ready for another look. |
@azadoks thanks a lot for such a nice tool.
In this PR I try to add the "O" part of this IO package to upf2 format.