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

Fixed reading of n, l, zeta in hsx_read_specie1 #462

Merged
merged 1 commit into from
May 3, 2022

Conversation

ahkole
Copy link
Contributor

@ahkole ahkole commented May 3, 2022

@juijan and I noticed that the reading of the quantum numbers of the orbitals (n, l, zeta) from the HSX file does not work correctly for HSX file version 1. There is an inconsistency between the ordering of the number during reading and writing. The writing routine (write_hsx in io_hsx.F90) writes them in the order n(1), l(1), zeta(1), n(2), l(2), zeta(2), ..., while the reading routine expects it in the order n(1), n(2), ..., l(1), l(2), ..., zeta(1), zeta(2), .... This PR fixes the reading routine to use the same ordering.

@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #462 (1f528ba) into main (fbe65bf) will increase coverage by 0.01%.
The diff coverage is 96.22%.

@@            Coverage Diff             @@
##             main     #462      +/-   ##
==========================================
+ Coverage   86.81%   86.83%   +0.01%     
==========================================
  Files         346      346              
  Lines       44107    44134      +27     
==========================================
+ Hits        38293    38322      +29     
+ Misses       5814     5812       -2     
Impacted Files Coverage Δ
sisl/io/siesta/fdf.py 77.68% <60.00%> (ø)
sisl/io/gulp/fc.py 100.00% <100.00%> (ø)
sisl/io/siesta/binaries.py 85.68% <100.00%> (-0.06%) ⬇️
sisl/physics/electron.py 77.20% <100.00%> (+0.14%) ⬆️
sisl/physics/sparse.py 88.60% <100.00%> (+0.39%) ⬆️
sisl/physics/tests/test_hamiltonian.py 100.00% <100.00%> (ø)
sisl/geom/nanotube.py 95.83% <0.00%> (+1.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5c1b45...1f528ba. Read the comment docs.

Copy link
Owner

@zerothi zerothi left a comment

Choose a reason for hiding this comment

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

Great! And thanks for testing!!!

@zerothi zerothi merged commit 96fed66 into zerothi:main May 3, 2022
@zerothi
Copy link
Owner

zerothi commented May 5, 2022

@ahkole something unrelated, I think your email address on your local machine does not match your github accounts email address.

The point is that github cannot recognize your commits and thus you will not be attributed in the github history (I would prefer to acknowledge your work, but I can't do anything here...)
See e.g. here https://github.com/zerothi/sisl/pull/462/commits where the commit author says A. H. Kole and not your username as is customary.

FYI!

@ahkole
Copy link
Contributor Author

ahkole commented May 5, 2022

@zerothi yes, that may be correct. I usually commit using my university email address but I created this GitHub account a long time ago with my personal email address so that's probably responsible for the mismatch. But I wasn't aware that GitHub then cannot attribute commits to my account, thanks for letting me know!

I will try to add my university address as a secondary email to my account and hopefully GitHub will then be able to properly recognize the commits.

@zerothi
Copy link
Owner

zerothi commented May 5, 2022

Yeah, it isn't exactly clear to me how this works, but I think you could check it by making a fake branch and commit some things there, then see if your user-name pops up.

@ahkole
Copy link
Contributor Author

ahkole commented May 5, 2022

I think it even works retroactively because I just added my university email to my account and now this commit https://github.com/zerothi/sisl/pull/462/commits does show a proper link to my account :)

@zerothi
Copy link
Owner

zerothi commented May 5, 2022

Even better! Great!

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