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

ProjwfcParser: fixed a bug when parsing pdos files #165

Merged
merged 1 commit into from
Jun 19, 2018

Conversation

zhubonan
Copy link
Contributor

@zhubonan zhubonan commented Jun 16, 2018

Fixes #166

Current implementation relying on the order of pdos files to be consistent with that of projections in <prefix>.out. However, names of <prefix>.pdos_atom#xxx files with array of pdos were not sorted correctly, giving wrongly associated PDOS arrays. Natural sorting is now used to get the correct order of files.

Also, I removed one line of unnecessary if compare.

@sphuber
Copy link
Contributor

sphuber commented Jun 19, 2018

@zhubonan thanks for the fix. As you noticed, there is an incompatibility with the current develop branch of aiida-core and the current release of aiida-quantumespresso. For this reason we have created the release_v2.0.2 and release_v3.0.0a1 branches. The former will be compatible with aiida-core==0.12.*, whereas release_v3.0.0a1 will be compatible with aiida-core v1.0.0a1 and up. So if you could please rebase this PR onto release_v2.0.2 branch, then I will make sure that it gets released in the patch release and after it will also be merged into release_v3.0.0a. Thanks

Current implementation relying on the order of pdos files to be
consistent with that of projections in out_file. However,
the the `<prefix>.pdos_atom#xxx` file were not sorted correctly.
Natural sorting should be used to get the correct order of
files.
@zhubonan zhubonan changed the base branch from develop to release_v2.0.2 June 19, 2018 10:38
@zhubonan
Copy link
Contributor Author

@sphuber No problem I have rebased into the release_v2.0.2 branch.

Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

Since this version does not support aiida-core version newer than 0.12.* anyway, there is no point in having the try catch

remote_folders = self.get_outputs(node_type=RemoteData)
# Maintain compatibility with aiida_core 0.12.0 and older versions
try:
remote_folders = self.get_outputs(type=RemoteData)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this try/catch. This version will only support 0.12.* and older anyway. In the v3* series we will make sure to update the signature for aiida-core v1.0.0 and higher.

Copy link
Contributor Author

@zhubonan zhubonan Jun 19, 2018

Choose a reason for hiding this comment

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

Sorry, I get confused when this change of method signature was made - it was there since 0.11.2. The try was for maintain compatibility with versions older than that and not related to v1.0.0 and higher.
Here is the commit that made the change aiidateam/aiida-core@991596e.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have remove this commit.

@sphuber sphuber merged commit 3a3fe52 into aiidateam:release_v2.0.2 Jun 19, 2018
@sphuber
Copy link
Contributor

sphuber commented Jun 19, 2018

@zhubonan thanks a lot for your contribution

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