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

New naming convention #7

Merged
merged 4 commits into from
Nov 29, 2024
Merged

New naming convention #7

merged 4 commits into from
Nov 29, 2024

Conversation

mfherbst
Copy link
Member

@mfherbst mfherbst commented Nov 22, 2024

Closes #1

  • Document new naming convention

@mfherbst
Copy link
Member Author

I had a go at the new naming convention for the pseudo families as we discussed. Here is my suggestion:

https://github.com/JuliaMolSim/PseudoPotentialData.jl/blob/e96df370200a9e06216c79a340801c4924a0d12e/src/pseudofamily.jl#L44--L53

Compared to your suggestion for files I made the following changes:

  • I did not like that "dojo" was so hidden. I think when ordering folders it makes sence to have this "catalogue" identifier be the first thing
  • You abbreviate "standard" as "std", but it is non-obvious what to use to abbreviate "stringent", so I decided to abbreviate neither of them.
  • It was not clear to me where to put "sr" or "fr". Since this makes a big difference in the pseudo (similar to the functional), I placed it after the type ("nc" or "paw").

@unkcpz @azadoks What do you think ?

@unkcpz
Copy link
Member

unkcpz commented Nov 26, 2024

It was not clear to me where to put "sr" or "fr"

Yes, this is an important information. I don't have strong opinion on putting it before or after functional type, so both are fine to me. But I think it is better to have also tag for non-relativistic say "nr"?

I did not like that "dojo" was so hidden. I think when ordering folders it makes sence to have this "catalogue" identifier be the first thing.

But the pseudopotential is not tight to library and we are kind of free to mix the use of them from different libraries and that is how SSSP works. Then the library is just how people curated the pseudos to their standard a tag not very much reflect the physical property of the pseudopotential.
I also see your point that for a DFT code it is easy for user to just pick the catalogue and use it without too much thinking. Then how about in this repo it maintains a mapping for libraries it will keep on support? Another thing to keep in mind is the SSSP is supposed to keep on update and may pick the other pseudo for certain elements in the future, in this way only the mapping need to update.
If we agree on there are three fields in front of library name, then to parse the library for category purpose should be doable.

You abbreviate "standard" as "std", but it is non-obvious what to use to abbreviate "stringent", so I decided to abbreviate neither of them.

Right, it seems better. I'll synchronous with this naming for pseudodojo pseudos in SSSP v2.

@mfherbst
Copy link
Member Author

But I think it is better to have also tag for non-relativistic say "nr"?

I don't think that exists, but I may be wrong. The point is that pseudos can always take care of scalar relativistic effect, or not ? In any case, would work with this naming.

I did not like that "dojo" was so hidden. I think when ordering folders it makes sence to have this "catalogue" identifier be the first thing.

But the pseudopotential is not tight to library and we are kind of free to mix the use of them from different libraries and that is how SSSP works.

Ok but the for us the prefix here would be sssp. Because this key is for selecting the library.

I also see your point that for a DFT code it is easy for user to just pick the catalogue and use it without too much thinking. Then how about in this repo it maintains a mapping for libraries it will keep on support?

Indeed that's the idea. So I capture you generally agree with the naming. If yes, the I finish and merge this.

If we agree on there are three fields in front of library name, then to parse the library for category purpose should be doable.

In principle yes, but I guess having some form of metadata is better. Thats my plan in this repo with the PseudoLibrary struct.

@unkcpz
Copy link
Member

unkcpz commented Nov 27, 2024

The point is that pseudos can always take care of scalar relativistic effect, or not ?

Take ld1.x as example https://www.quantum-espresso.org/Doc/INPUT_LD1.html, rel=0, 1, 2 are for nr, sr, fr. But it will work with the naming anyway.

Indeed that's the idea. So I capture you generally agree with the naming. If yes, the I finish and merge this.

To me the naming in this repo is fine. Do you want me to synchronous with this for SSSP v2 or it is okay to switch the order of fields? Just let me know and I'll double check with Nicola on Friday.

@mfherbst
Copy link
Member Author

To me the closer the naming between SSSP and this library, the better.

I understand now, that it will likely not map perfectly, but ideally the naming should not be too outrageously different across the ecosystems to make it easy for people to use one if they know the other.

@mfherbst
Copy link
Member Author

Ok, I'll wait with the merge until Friday then.

@mfherbst mfherbst marked this pull request as ready for review November 27, 2024 10:26
@mfherbst mfherbst merged commit 7e60079 into master Nov 29, 2024
5 checks passed
@mfherbst mfherbst deleted the newnames branch November 29, 2024 20:01
@mfherbst
Copy link
Member Author

@unkcpz I interpret your silence as agreement that the current scheme is in no stark contrast to your discussion with Nicola.

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.

Overhaul of pseudofamily identifiers and tarballs
2 participants