Skip to content

Conversation

@moritzgubler
Copy link
Contributor

This branch implements GTH pseudopotentials with non linear core correction.

We had to implement a new initial guess mechanism that uses numeric atom centered orbitals to start pseudopotential calculations. It also works for all electron calculations.

Let me know if you are interested in merging these features, then I will clean up everything and add some more rigorous testing.

At the moment only OMP is tested, I never checked the results obtained with MPI.

@gitpeterwind
Copy link
Member

gitpeterwind commented Dec 5, 2025

@ilfreddy @gitpeterwind I think the pseudopotential implementation works correct after merging master into this branch. But by testing this branch I found that ZORA and AZORA calculations are not converging on the master branch and now also on this branch: #526

Thanks for testing this!
I think I found the reason and a fix is included in my latest PR. #524

@moritzgubler
Copy link
Contributor Author

@ilfreddy @gitpeterwind we tested the rebased version of the pseudopotential implementation and the results are now consistent with our previous tests. So it is ready from my side.

#include "qmfunctions/qmfunction_fwd.h"
#include <string>

/** @file sad.h
Copy link
Contributor

Choose a reason for hiding this comment

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

sad?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

return result;
}

// I had to implement this functions, thez return not meaningful values.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do they have t have a return value if it is not meaningful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because the interface demands that I implement it. added a more comprehensive docstring

Copy link
Contributor

Choose a reason for hiding this comment

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

There is basically no documentation in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now there is:)

Copy link
Contributor

Choose a reason for hiding this comment

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

We had a discussion about documentation at a group meeting in the fall and we agreed that all documentation, should generally go in the header files. In the implementation files, one should add documentation if the implementation of a particular routine/method is difficult to understand just by looking at the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Docs are now in header file

@ilfreddy
Copy link
Contributor

ilfreddy commented Jan 6, 2026

I believe this can basically go. It lacks a bit of documentation, and in some cases the documentation should be moved to the header files. This was discussed in a group meeting here in Tromsø and the majority was for documenting header files.
@robertodr @stigrj Could you have a look. I believe @moritzgubler has been waiting long enough for this to be approved :-)

@ilfreddy
Copy link
Contributor

Thanks for keeping this up-to-date @moritzgubler (I know it is a lot of work!). There is one more PR in the pipeline (#481 from @msnik1999). As soon as that one goes in, yours is next!

@moritzgubler
Copy link
Contributor Author

No worries, I am working on integrating your feedback now.

@ilfreddy
Copy link
Contributor

ilfreddy commented Jan 23, 2026

No worries, I am working on integrating your feedback now.

Converted to draft! Just remove the flag as soon as you are done 😃

@ilfreddy ilfreddy marked this pull request as draft January 23, 2026 12:46
@moritzgubler
Copy link
Contributor Author

moritzgubler commented Jan 23, 2026

@ilfreddy @gitpeterwind The energies from the na_pp test have changed, I guess because of #532
etot: -2.460821614906e -> -2.460024562757
eigenvalue: -0.111452491089 -> -0.107870374722
ekin: 0.076295587117 -> 0.070570044776

Do you expect changes of this magnitude?

@moritzgubler moritzgubler marked this pull request as ready for review February 3, 2026 08:12
@moritzgubler moritzgubler marked this pull request as draft February 3, 2026 08:13
@ilfreddy
Copy link
Contributor

ilfreddy commented Feb 4, 2026

I believe this is now good to go. Any reason why you marked it as draft again?

@moritzgubler moritzgubler marked this pull request as ready for review February 5, 2026 07:52
@moritzgubler
Copy link
Contributor Author

I wanted to add a manual. Now it's ready again from my side

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.

4 participants