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

[BUG] 'get_abacus_STRU' is not consistent with 'get_coords' in 'dpdata/abacus/scf.py' #1711

Closed
AsymmetryChou opened this issue Jan 27, 2025 · 3 comments · Fixed by #1714
Closed
Assignees
Labels
bug Something isn't working

Comments

@AsymmetryChou
Copy link

AsymmetryChou commented Jan 27, 2025

Bug summary

When I run DP-GEN using abacus for fp, an error exits:
File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/generator/lib/abacus_scf.py", line 364, in get_abacus_STRU atom_names, natoms, types, coords = get_coords(celldm, cell, geometry_inlines) ValueError: too many values to unpack (expected 4)

get_coords, the function in dpdata, output the following variables(corresponding to the version in Github-Master ):
return atom_names, atom_numbs, atom_types, coords, move, mags

Evidently, they are not consistent. 4 variables in DPGEN, but 6 variables in dpdata.

It seems that there is a Version Conflict?

The version info:

Image

DP-GEN Version

0.11.1

Platform, Python Version, Remote Platform, etc

No response

Input Files, Running Commands, Error Log, etc.

Error Log:
Traceback (most recent call last): File "/opt/deepmd-kit-2.2.1/envs/dpmd/bin/dpgen", line 8, in <module> sys.exit(main()) File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/main.py", line 233, in main args.func(args) File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/generator/run.py", line 5109, in gen_run run_iter(args.PARAM, args.MACHINE) File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/generator/run.py", line 4458, in run_iter make_fp(ii, jdata, mdata) File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/generator/run.py", line 3520, in make_fp make_fp_abacus_scf(iter_index, jdata) File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/generator/run.py", line 3298, in make_fp_abacus_scf _link_fp_abacus_pporb_descript(iter_index, jdata) File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/generator/run.py", line 3052, in _link_fp_abacus_pporb_descript stru_param = get_abacus_STRU("STRU") File "/opt/deepmd-kit-2.2.1/envs/dpmd/lib/python3.8/site-packages/dpgen/generator/lib/abacus_scf.py", line 364, in get_abacus_STRU atom_names, natoms, types, coords = get_coords(celldm, cell, geometry_inlines) ValueError: too many values to unpack (expected 4)

Steps to Reproduce

run DP-GEN in Bohrium notebook 从 DFT 到 MD|超详细「深度势能」材料计算上手指南.

In charpter 4 : dpgen run param_abacus.json machine.json

Further Information, Files, and Links

No response

@AsymmetryChou AsymmetryChou added the bug Something isn't working label Jan 27, 2025
@njzjz
Copy link
Member

njzjz commented Jan 27, 2025

@pxlxingliang It seems to me that the private API should not be used. Or if it is a public API, there should be no breaking change.

@pxlxingliang
Copy link
Contributor

@pxlxingliang It seems to me that the private API should not be used. Or if it is a public API, there should be no breaking change.

We should not use this dpdata API, I will fix this bug.

wanghan-iapcm pushed a commit to deepmodeling/dpdata that referenced this issue Feb 14, 2025
Refactor the codes to read and write ABACUS/STRU, and move the functions
in a single file abaucs/stru.py

Now, now using dpdata.system to read an ABACUS STRU will also return
below informations in data dict:
```
{
           "masses": list of atomic masses,
            "pp_files", list of pseudo potential files,
            "orb_files", list of orbital files,
            "dpks_descriptor": the deepks descriptor file,
}
```
And, these information can also be written to a new STRU file
automatically.


Later, I will based on this commit to fix the bug in dpgen
deepmodeling/dpgen#1711


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Introduced a dedicated module for structure file handling, enhancing
the parsing and conversion of lattice, species, and coordinate data.

- **Refactor**
- Streamlined data extraction processes for simulation and relaxation
workflows, reducing redundant operations and improving error clarity.
- Updated plugin methods to leverage the enhanced structure processing
functions for improved efficiency.

- **Tests**
- Improved test setups and cleanups, ensuring consistent handling of
structure files and robust validation of the new parsing logic.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: root <pxlxingliang>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@njzjz njzjz linked a pull request Feb 14, 2025 that will close this issue
@njzjz njzjz marked this as a duplicate of #1717 Feb 15, 2025
@NKJunhongLi
Copy link

Hello, developers. Today I find that dpdata is upgraded to version 0.2.23 in pip. The bug of reading ABACUS STRU still exist. There are great changes in dpdata code. So many functions are modified and now it even occurs error during import ... when running dpgen. Maybe it is also necessary for updating the dpgen and apex-flow codes?

wanghan-iapcm pushed a commit that referenced this issue Feb 21, 2025
Fix the bug induced by the update of dpdata.
Now the reading and writing of ABACUS STRU is by a more general
dpdata/abacus interface, and remove some redundant codes.

This commit is based on dpdata commit
deepmodeling/dpdata#793, and it is valid after
that commit is merged.

related issue:
#1711

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Refactor**
- Streamlined the workflow for generating atomic structure data by
removing redundant parameters and centralizing functionality.
- Improved handling of atomic data structures for more efficient
processing and organization.
- **Tests**
- Enhanced the precision of validations by refining comparisons to focus
on key attributes, ensuring more robust result accuracy.
	- Corrected the method name for improved clarity in the test suite.
- Added a new key to the test data structure for enhanced functionality
in subsequent tests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: root <pxlxingliang>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@njzjz njzjz closed this as completed Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants