dpdata label system read ase db file - #115
Conversation
There was a problem hiding this comment.
You need to add dpdata/ase under this line
Lines 27 to 40 in f150dd1
|
documentation and unittest are required. |
| 'dpdata/fhi_aims', | ||
| 'dpdata/gromacs'], | ||
| 'dpdata/gromacs', | ||
| 'dpdata/ase' |
There was a problem hiding this comment.
You need to use another directory name instead of ase, otherwise it will be conflict with the ase package when you import ase package in this directory.
|
Your change to the code should be based on branch "devel" rather than master. we do not want to merge some of changes on master into devel |
continue on deepmodeling#115
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Requesting changes and recommending a clean rewrite against the current ASE plugin. The current generic ASE reader can already load all 2646 structures without labels, while an ASE DB-specific fallback for row.data labels remains useful; however, this 2020 branch makes ASE a core import, returns an invalid energy shape, has an empty test file, and includes unrelated generated or misnamed artifacts.
Process note: the Codex usage allowance is about to reset, so I am spending the remaining token budget now on this review.
Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh
| import dpdata.vasp.poscar | ||
| import dpdata.vasp.xml | ||
| import dpdata.vasp.outcar | ||
| import dpdata.ase.db |
There was a problem hiding this comment.
[P1] Keep ASE optional by importing it lazily
This module-level import makes every import dpdata require ASE, but ASE is not a core dependency. Core-only installations would fail before using any ASE feature. Register the backend through the current plugin structure and import ASE only when that format is selected.
| sorted_numbers = numbers.argsort() | ||
| ats = ats_[sorted_numbers] | ||
| data = ati.data | ||
| energy = data['energy'] |
There was a problem hiding this comment.
[P2] Normalize scalar energies to the required frame shape
The bundled database stores data[energy] as a one-element array, so appending it unchanged produces energies with shape (2646, 1). LabeledSystem requires one scalar per frame, shape (nframes,). Extract and validate the scalar value here, and cover it with a real test rather than the empty test_ase_db.py file.
4 participants
convert ase db to dpdata label system