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

[API] Add function for parsing hubbard.dat style files #933

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mtaillefumier
Copy link
Collaborator

Add the possibility to parse QE hubbard.dat file directly with SIRIUS. It is useful for cp2k.

@mtaillefumier
Copy link
Collaborator Author

Please wait my signal before merging even if everything is OK.

namespace sirius {

static std::vector<std::string>
split(std::string str, std::string token)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it will. I will use it.

parse_atom_string(std::string& at_str__) -> std::tuple<std::string, int, int>
{
std::vector<std::string> split_str_ = split(at_str__, "_");
int n_ = -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why underscore in local variables? it is not needed

// a bit pedantic....
n_ = std::stoi(split_str_[1]);

switch (split_str_[1][1]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a code repetition with https://github.com/electronic-structure/SIRIUS/blob/develop/src/unit_cell/atom_type.cpp#L412

better solution: introduce a pure function that returns l by charachter label. It can be defined for example in atom_type.hpp

@@ -196,6 +196,11 @@ class Hubbard
}
};

extern void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this are normal functions on the c++ side. No raw pointers as arguments

}

void
add_hubbard_atom_pair(Simulation_context& ctx__, int* const atom_pair__, int* const translation__, int* const n__,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no pointers as arguments

}

void
parse_hubbard_file(Simulation_context& ctx__, const std::string& data_file__)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our convention is T const& arg

}

if ((split_string[0] == "U") || (split_string[0] == "u")) {
const double U_ = std::stod(split_string[2]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local variables should not have underscore

const double V_ = std::stoi(split_string[5]);
int translation1[3] = {0, 0, 0};
int atom_pair_[2];
int translation2[3] = {0, 0, 0};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be r3::vector T;

@mtaillefumier
Copy link
Collaborator Author

Actually there is a lot more work coming for this.

@mtaillefumier mtaillefumier marked this pull request as draft December 19, 2023 10:43
@mtaillefumier mtaillefumier self-assigned this Dec 19, 2023
@mtaillefumier
Copy link
Collaborator Author

I need to finish this PR.... Will see what I can do next week.

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.

2 participants