Fix missing stress unit conversion (kBar->eV/A^3) in json_to_extxyz.py
#61
Conversation
import col names from matbench_discovery.preds
for more information, see https://pre-commit.ci
…batch and run_train.py into train_mace.py and avoid shell scripts where possible. what about setup.sh?
remove dict deepcopy
chiang-yuan
force-pushed
the
fix/stress-units
branch
from
9fad2fe to
7ffaf4e
Compare
chiang-yuan
force-pushed
the
fix/stress-units
branch
from
70fc8b3 to
7879647
Compare
chiang-yuan
force-pushed
the
fix/stress-units
branch
from
bf61c88 to
627c705
Compare
models/mace/json_to_extxyz.py
Outdated
|
|
||
| block = json_data[material_id][trajectory_id] | ||
| for trajectory_id in json_data[material_id]: | ||
| block = copy.deepcopy(json_data[material_id][trajectory_id]) |
There was a problem hiding this comment.
Is copy.deepcopy really needed? This might slow down the conversion substantially. Is it to avoid errors on processing twice? Maybe better to keep a dict that maps processed IDs to processed results. That way you can check in constant time whether you should skip an ID.
There was a problem hiding this comment.
since I use dict.pop the error will raise if size of dictionary changes within a for loop. copy will avoid that. The speed is doable for now it is roughly one hour processing time
There was a problem hiding this comment.
Always good to leave why comments in the code. Why are you doing something (if it's not obvious).
2nd question: Why are you using pop? What's wrong with get?
There was a problem hiding this comment.
we don’t want repeated forces, stress, magmoms, energy show up in atoms.info
There was a problem hiding this comment.
I don't follow. get also returns None?
There was a problem hiding this comment.
Yes, but get will not remove the key value pair from the dictionary and those will all go into atoms.info
matbench-discovery/models/mace/json_to_extxyz.py
Lines 60 to 61 in af1d7ad
There was a problem hiding this comment.
I see. Let's just use set arithmetic to exempt those keys from double processing then.
janosh
changed the title
json_to_extxyz.pyjson_to_extxyz.py
No description provided.