Skip to content

Conversation

@JunnHuo
Copy link
Collaborator

@JunnHuo JunnHuo commented Jul 10, 2025

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jul 10, 2025

CLA assistant check
All committers have signed the CLA.

@leeleolay leeleolay self-requested a review July 11, 2025 03:31
Copy link
Collaborator

Choose a reason for hiding this comment

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

此文件内容重点抽象了predictor,建议进一步抽象为predictor组件,新建一个ppmat/predictor文件下,放到该目录下,可兼容加载属性预测模型,机器学习势函数模型的多个任务的predictor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已修改为predict组件中

Copy link
Collaborator

Choose a reason for hiding this comment

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

此文件放到calculator下面会不会更好一点,calculator支持md的计算功能

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已修改为task组件中

Copy link
Collaborator

Choose a reason for hiding this comment

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

不建议使用task,和场景的计算“任务”概念冲突

Copy link
Collaborator

Choose a reason for hiding this comment

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

这里修改为ase_engine会不会更合适一点,方便后面兼容多个计算软件库,从calculor的功能层面,支持md,dft任务,从支持这些任务的引擎来说,包括了ase,或者以后添加的其他软件库

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个同md,是不是也可以迁移到calculator中

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已修改

Copy link
Collaborator

Choose a reason for hiding this comment

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

md_ase_case这里的代码看起来和md有些重复,功能区分和调用逻辑上是不是可以再优化一下

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已删除,合并到一个文件中

Copy link
Collaborator

Choose a reason for hiding this comment

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

这里同md_case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已删除,合并到一个文件中

@leeleolay
Copy link
Collaborator

predictor和calculator建议取消掉ppmat,和trainer保持类似的命名一致性规则

import sys

__dir__ = os.path.dirname(os.path.abspath(__file__)) # ruff: noqa
sys.path.insert(0, os.path.abspath(os.path.join(__dir__, "../../"))) # ruff: noqa
Copy link
Collaborator

Choose a reason for hiding this comment

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

使用新的安装方式后,不需要动态引入路径了,可以删除这两行代码了

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已删除

if "energy_per_atom" in label_names and "energy" not in label_names:
label_names.append("energy")
else:
raise NotImplementedError(
Copy link
Collaborator

Choose a reason for hiding this comment

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

最好也支持一下mattersim

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这版pr后支持mattersim

Copy link
Collaborator

@zhiminzhang0830 zhiminzhang0830 left a comment

Choose a reason for hiding this comment

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

辛苦后续需修改模型readme中关于predict部分的运行命令,其他说明文件中如涉及predict部分辛苦也一并修改

out = self.model.predict(data)
else:
out = self.model.predict(data)
out = self.post_process(out)
Copy link
Collaborator

Choose a reason for hiding this comment

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

建议将此处的model.predict、post_process等操作整合到一个新的函数内,并在此处调用,也方便在ase_interface中调用

save_load.load_pretrain(model, self.checkpoint_path)
else:
logger.info("Since model_name is given, downloading it...")
model, config = build_model_from_name(self.model_name, self.weights_name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

if分支中对chgnet修改了其is_intensive参数,但else分支中未做处理,可能会导致此处产生的模型不符合预期

def load_inference_model(
self,
ase_calc: bool = False,
device: Optional[str] = "cpu",
Copy link
Collaborator

Choose a reason for hiding this comment

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

device参数在以下的代码中未使用,需要增加使用该参数的代码

all_files = [
osp.join(file_path, f)
for f in os.listdir(file_path)
if f.endswith(".cif")
Copy link
Collaborator

Choose a reason for hiding this comment

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

此处与注释中的不太一致,应修改注释或增加其他文件类型的支持

Supported formats include: CIF, POSCAR/CONTCAR, CHGCAR, LOCPOT, vasprun.xml, CSSR, Netcdf and pymatgen's JSON-serialized structures.

Copy link
Collaborator

@leeleolay leeleolay left a comment

Choose a reason for hiding this comment

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

建议模块从ppmat/predict修改为ppmat/predictor

@leeleolay
Copy link
Collaborator

predictor和calculator逻辑拆分的还是有点问题

Copy link
Collaborator

@leeleolay leeleolay left a comment

Choose a reason for hiding this comment

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

文件名Applications 建议修改为 “Data–Mechanics Integrated Scientific Computing”

hydra:
job:
name: potential
chdir: True # 是否切换工作目录到 run.dir
Copy link
Collaborator

Choose a reason for hiding this comment

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

中文注释辛苦切换为英文

Copy link
Collaborator

Choose a reason for hiding this comment

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

test下的文件如果不是必须的辛苦不要提交了

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件是不是提交错了

Copy link
Collaborator

@leeleolay leeleolay left a comment

Choose a reason for hiding this comment

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

新增的training yaml建议都放到Training_test里面

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件的作用是什么?如果是结果文件的话可以删除掉,或者在readme文件里注明参考结果文件,相关文件放到网上存储

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件可以删除

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个地方的property是用在哪个地方的?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sample这块的都可以先删掉

path (str): Path to the data.
"""
json_data = read_json(path)

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个地方的改动是为什么?

Copy link
Collaborator

Choose a reason for hiding this comment

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

predictor里面为什么要暴露StructureSampler

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个可以删掉的

Copy link
Collaborator

Choose a reason for hiding this comment

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

这几个文件如果没有改变辛苦可以不提交

Copy link
Collaborator

Choose a reason for hiding this comment

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

这里和mattersim为什么出现了两个readme文件?

@leeleolay
Copy link
Collaborator

辛苦提供一个readme文档,怎么运行;requirement里添加hydra

@leeleolay leeleolay added the enhancement New feature or request label Sep 30, 2025
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个地方还原吧

Copy link
Collaborator

Choose a reason for hiding this comment

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

这里请还原

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件夹应该还原不改动

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件夹请还原

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件夹请还原

Copy link
Collaborator

Choose a reason for hiding this comment

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

请还原

property_names (Optional[str], optional): A comma-separated list of
target property names to predict. Defaults to "formation_energy_per_atom".
data_norm_mean (float, optional): The mean used for normalizing target values.
data_mean (float, optional): The mean used for normalizing target values.
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里为什么这么改

tensor=paddle.to_tensor(data_norm_std), name="data_norm_std"
)
self.register_buffer(tensor=paddle.to_tensor(data_mean), name="data_mean")
self.register_buffer(tensor=paddle.to_tensor(data_std), name="data_std")
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里为什么取消锁进

formation_energy_per_atom:
band_gap:
__class_name__: paddle.nn.L1Loss #MAEMetric
__init_params__: {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里为什么要修改

@@ -0,0 +1,21 @@
# Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

2023 改为2025

@@ -0,0 +1,245 @@
# Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

修改为2025

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件是不是可以还原

Copy link
Collaborator

Choose a reason for hiding this comment

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

添加paddle 权利说明 注释

@@ -0,0 +1,452 @@
# Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

2025

Copy link
Collaborator

@leeleolay leeleolay left a comment

Choose a reason for hiding this comment

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

dataset的代码还原一下吧或者为什么要修改,之前的问题回复一下

from different matbench JSON files and processing them for materials property prediction.
science benchmark datasets.
The implementation supports loading multiple properties from different
matbench JSON files and processing them for materials property prediction.
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants