Basis Set Parsers and Test File#11
Conversation
vonDonnerstein
left a comment
vonDonnerstein
left a comment
There was a problem hiding this comment.
Please update your pull request (by pushing corresponding commits to sasadada:master).
| @@ -0,0 +1,635 @@ | |||
| module BasisSetModule | |||
| export BasisSet, readTX93, readGaussian, readGAMESSUS, readNWChem, readDalton | |||
There was a problem hiding this comment.
can we use readBasisSetTX93, readBasisSetGaussian, etc. to keep the naming convention intact?
| end | ||
|
|
||
| # Element Dictionary : Full name to Symbol | ||
| ElementSymbols = Dict{String,String}( |
There was a problem hiding this comment.
To me this seems to be functionally connected to the Element type defined in Atom.jl. Can we move this there and create an additional Creator-method based on this?
|
|
||
| function dict(children) | ||
| dicty = Dict() | ||
| for i = 1:length(children) |
There was a problem hiding this comment.
Could you comment on your reason for breaking with the indentation scheme? As you are breaking indentation throughout this document, maybe you could adjust wherever it isn't necessary.
| # Supplementary Functions for the PEG BasisSet Parsers | ||
|
|
||
|
|
||
| function dict(children) |
There was a problem hiding this comment.
As the name doesn't appear self-explanatory to me, could you supply documentation (https://docs.julialang.org/en/stable/manual/documentation/)?
| return dicty | ||
| end | ||
|
|
||
| function DictElements(children) |
There was a problem hiding this comment.
As the name doesn't appear self-explanatory to me, could you supply documentation (https://docs.julialang.org/en/stable/manual/documentation/)?
| return bases | ||
| end | ||
|
|
||
| function SPOrbitals(children) |
There was a problem hiding this comment.
While I would assume that this function is separating Shared-SP basis functions, could you still supply documentation (https://docs.julialang.org/en/stable/manual/documentation/) to be on the safe side?
| return Primitives | ||
| end | ||
|
|
||
| function dictDalton(children) |
There was a problem hiding this comment.
As the name doesn't appear self-explanatory to me, could you supply documentation (https://docs.julialang.org/en/stable/manual/documentation/)?
| end | ||
|
|
||
|
|
||
| function basesDalton(children) |
There was a problem hiding this comment.
As the name doesn't appear self-explanatory to me, could you supply documentation (https://docs.julialang.org/en/stable/manual/documentation/)?
| end | ||
|
|
||
|
|
||
| function CreateCorrectPrimitives(children) |
There was a problem hiding this comment.
Following the naming conventions (http://schurkus.com/2015/12/05/quantumlab-design-guidelines/), I would expect this function to be computePrimitives ("correct" seems redundant, because who would be interested in "erroneos" Primitives). I'd assume that this function takes a list of Nodes (btw. could you use Type assertions (https://docs.julialang.org/en/stable/manual/methods/#Defining-Methods-1) here?) and extracts all PrimitiveGaussianBasisFunctions from them and their children recursively. If this is not the aim of this function maybe a totally different name might be much more explanatory?
| return LinearFactor | ||
| end | ||
| ############################################################################################################################################ | ||
| # Main Functions |
There was a problem hiding this comment.
I'd like to go over these functions in detail. This will probably be much easier once you have made the other adjustments, so I'll wait until then. Same holds for runtests.jl below. Thank you for updating your pull request (simply push your changes to sasadada:master).
|
Hi Ivan, |
|
Hi Henry, |
|
Hi Ivan, are there still changes you wanted to make? I assume you don't want to spend too much more time on this project, so if not, I would suggest you just add tests for your functionalities to |
|
Also, it would be great, if you could make sure to have Allow edits from maintainers checked on the right of this conversation. Thanks. |
3 participants
No description provided.