Conversation
* 提交了模型结构
* 增加了原论文中使用的两个数据集相关代码
* Remove duplicate import
Removed duplicate import of OMol25Dataset.
* 根据Review要求，添加版权声明
* 移动loss和metrics到对应的位置
* 完成utils工具迁移
* 模型进一步对齐
* 对齐模型权重
* 规范化数据集格式并全部测试通过
* 增加了数据集对fp64的支持
* 修改模型支持fp64
* 修复了fp32漏网之鱼
* fix some import
* add ECFormer train code
* translate Chinese to English
* Optimize training algorithm
* 根据建议调整目录结构
* Add README for ECFormer model and spectrum prediction task
* SphereNet: complete PR review overhaul (15 comments)
- QM9: all 12 properties with three-stage config format
- MD17: 8 molecules with energy+force prediction
- MD17Dataset: .npz download, fixed train/val/test splits
- QM9Dataset: split parameter for internal subset selection
- DefaultCollator: merged NodeBatchCollator via heuristic detection
- Graph building: radius_graph in ppmat/utils/graph_utils.py
- Geometry: xyz_to_dat in ppmat/utils/xyz_utils.py
- Scatter: scatter_min using put_along_axis(reduce='amin')
- SphereNet: single-file model, public modules only
- SphereNetEmbedding/EdgeInitializer/EdgeUpdate/NodeUpdate/GraphUpdate
- Basis: spherical_fourier_bessel.py with local _real_sph_harm
- Initializer: glorot_orthogonal using Orthogonal + scaling
- README: dataset-first docs, results table without TODO markers
* README: align results table with MegNet format, add MD17 convergence results
* cleanup: remove ECD/IR contamination, restore DensityCollator, remove NodeBatchCollator alias
- Remove ECDMetrics/IRMetrics from metrics/__init__.py (source files absent)
- Remove IRDataset/ECDDataset from datasets/__init__.py (source files absent)
- Restore DensityCollator to base version (no modifications)
- Remove NodeBatchCollator = DefaultCollator alias
- Revert batch_size=2 back to batch_size=init_params["batch_size"]
- Clean commit history comments from collate_fn.py
* refactor(MD17): add bcebos mirror, MD5 check, fallback download
- Add bcebos mirror URL (fast download in mainland China) with MD5
- Use get_datasets_path_from_url for distributed-safe download
- Keep fallback to individual molecule URLs from quantum-machine.org
- Based on mp20 dataset pattern and reviewer suggestion
* refactor(config): align spherenet configs with existing project format
- Reorder sections: Global -> Trainer -> Model -> Metric -> Optimizer -> Dataset
- Add num_workers: 4 and use_shared_memory: False to each dataset split
- Fix build_dataloader to respect per-dataset num_workers/use_shared_memory
- Applies to all 19 QM9 and MD17 config files
* fix: restore upstream import formatting in collate_fn.py and datasets/__init__.py
* fix: remove duplicate InfGCN/MatENO imports in models/__init__.py
* fix: move num_workers/use_shared_memory to dedicated loader section in config
Previously these fields were placed inside the dataset block and extracted
via dataset_cfg.pop() — a non-standard hack. Now each split has a proper
loader: section (dataset → loader → sampler order), matching the project
convention used by MegNet and other models.
* fix: restore elif chain in collate_fn DefaultCollator
Merged node-batch detection into the elif chain and removed
redundant comments, matching upstream's elif pattern.
* refactor: migrate radius_graph to graph_converter as SphereNetRadiusGraph class
Moved from ppmat/utils/graph_utils.py (new file, upstream absent)
into ppmat/models/common/graph_converter.py as a class following
the project's graph converter convention (__call__ interface).
Deleted graph_utils.py.
* refactor(MD17): align dataset format with mp20 style
Moved url/md5 to class-level attributes, added transforms support
and **kwargs for compatibility, added STATS table to docstring.
Renamed private constants with _ prefix to avoid name collision.
* fix(collator): replace global DefaultCollator hack with dedicated SphereNetCollator
Reverted DefaultCollator to upstream version (no node-batch logic).
Added SphereNetCollator class that handles variable-length molecule
tensor batching. Updated all 20 spherenet configs to use
collate_fn: SphereNetCollator in the loader section.
This avoids modifying the global collation logic that could affect
other datasets.
* feat: per-molecule radius graph + energy/force prediction
Radius graph now processes each molecule independently to avoid
O(batch_N²) memory. max_num_neighbors documented but not enforced.
Energy+force: SphereNet._forward conditionally removes no_grad()
when energy_and_force=True for gradient-based force computation.
SphereNetPP.forward adds force via autograd and force L1 loss.
* fix: MD17 md5/bundle paths, radius_graph gather, force create_graph
- Fixed MD5 to match actual bcebos file (multipart upload ETag)
- Added bundle npz filename mapping for correct extraction
- Fixed paddle.gather usage in per-molecule radius graph
- Removed create_graph=True (Paddle's atan2 lacks 2nd-order grad) but force path verified
* address PR review: RadiusGraph rename, force fix, angle fix, QM9/MD17 fixes
- Rename SphereNetRadiusGraph → RadiusGraph, merge _radius_graph_impl
- [P0] Fix force branch: pos.stop_gradient + create_graph try/except
- [P1] Fix xyz_to_dat bond angle: use -vec_kj for correct k-j-i angle
- [P1] QM9: seed=42 split 110000/10000/10831 (align DIG/TFDS)
- [P1] QM9: return all property_names + apply transforms in raw_tensor
- [P1] QM9: validate property names, raise on unknown (no silent np.empty)
- [P1] MD17: add force L1Loss metric to all 8 configs
- Clean up stale layers/ directory
- Paddle 3.0 compat: bias/weight set_value instead of fill_
* fix: QM9 unified download, MD17 split/force_key
- QM9: use get_datasets_path_from_url with md5 verification
- MD17: split train=1000/val=1000/test=rest (DIG convention)
- MD17: force_key parameter in SphereNetPP (not hardcoded 'force')
* [SphereNet] RadiusGraph→独立函数; 数据集MP20模式+edge_index; MODEL_REGISTRY注释; 配置build_graph_cfg
- radius_graph() 独立函数放在 ppmat/models/common/radius_graph.py
- RadiusGraph类保留为薄封装(兼容build_graph_converter)
- SphereNet.forward()接受edge_index参数,不再内部调RadiusGraph
- QM9/MD17数据集: build_graph_cfg驱动预建edge_index, inline下载
- SphereNetCollator: edge_index batch偏移处理
- 所有yaml配置: dataset加build_graph_cfg
- MODEL_REGISTRY加SphereNet注释
* remove pretrained-weights comment from MODEL_REGISTRY (training already done)
* [SphereNet] scatter_min删除 + xyz_to_dat移入model + README论文图
* [SphereNet] QM9/MD17数据集按MP20模式重写
- 删除mode='packed'、ASE/pymatgen依赖、_ensure_raw_data等冗余方法
- 内联下载(get_datasets_path_from_url)
- build_graph_cfg驱动预建edge_index, pickle缓存, rank0+barrier
- __getitem__返回{z, pos, edge_index, properties}
- 配置yaml去掉mode: raw_tensor
* 修正论文链接
* remove paper page screenshot from README (not a proper figure)
* add Figure 1 (rendered from paper PDF) to README
* use user-provided SphereNet figure
* remove implementation notes from dataset docstrings
* replace README figure with vector SVG (Figure 4 from paper)
* Revise caption for SphereNet architecture image
Updated the caption for the SphereNet architecture image.
* tighter SVG crop to remove black borders
* remove RadiusGraph from __all__; move figure to docs/SphereNet.png
* reduce batch_size for parallel training (QM9=128, MD17=64)
* remove run_all.sh
* remove sed temp file
* reduce QM9 max_epochs to 100 for batch training
* fix QM9 md5 to lowercase
* fix QM9 data path to use existing cached data
* fix xyz_to_dat O(N²) memory blowup: use per-node segment search instead of full-batch expand
- Replace O(num_edges²) expand with per-source-node segment search
- Both triplets and quadruplets now use sorted segment iteration
- Enables training with batch_size=128 without OOM on A800 80G
* fix scalar indexing in xyz_to_dat to avoid CUDA misaligned address
* replace nonzero with where in xyz_to_dat to avoid CUDA misaligned address
* revert xyz_to_dat to original O(N²) version; failed rewrite caused CUDA errors
The per-node segment search rewrite hit CUDA misaligned address errors.
Keeping the original expand-based approach with reduced batch_size
(batch_size=32) instead, which avoids OOM while staying simple.
* remove threebody_indices.py stub — Cython module compiled via setup.py build_ext --inplace
The threebody_indices module is a Cython .pyx file that requires
compilation. The previous stub was incorrect; the proper fix is to
run 'python setup.py build_ext --inplace' to generate the .so file,
which is already gitignored.
* fix MD17 eval autograd: allow_unused=True for paddle.grad in no_grad context
eval_epoch uses self.no_grad_context_manager which wraps forward in
paddle.no_grad(), disconnecting pos from the backward graph.
Added allow_unused=True to paddle.grad() and handle None gradient,
so eval gracefully skips force computation when graph is unavailable.
* fix MD17 eval KeyError: add zero-force placeholder when graph unavailable
When eval runs with paddle.no_grad(), the force gradient cannot be
computed via paddle.grad (graph disconnected). Previously forces_pred
was None and skipped the force key in prediction dict, causing
KeyError in eval_epoch metric computation. Now zeros_like(pos) is
used as placeholder so the force key is always present.
* fix MD17 torsion gradient: avoid put_along_axis reduce=amin
Paddle's put_along_axis with reduce='amin' has a buggy backward pass
(PutAlongAxisGradNode). Replaced the two-step (amin + assign) selection
with argsort + unique + single assign, keeping gradients intact.
* fix MD17 torsion gradient: replace put_along_axis(amin) with scatter
Paddle's put_along_axis with reduce='amin' has a buggy backward pass.
Replaced with argsort + unique + scatter(overwrite=True), keeping
gradients intact through the torsion computation chain.
* fix torsion gradient: no_grad selection + direct indexing
Paddle's scatter/put_along_axis backward triggers PutAlongAxisGradNode
which can crash the force gradient computation. Solution: run the
best-angle-selection logic in paddle.no_grad(), then use direct tensor
indexing torsion[sel] = torsion_angle[keep] which has a clean gather
backward that Paddle handles correctly.
* fix torsion gradient: no_grad selection + scatter(overwrite) avoid inplace
Inplace assignment torsion[sel]=value breaks gradient flow. Use
paddle.scatter(overwrite=True) instead — functional, returns a new
tensor. The selection logic runs in paddle.no_grad() so the scatter's
backward doesn't trigger PutAlongAxisGradNode.
* fix scatter_sum dim_size bug for isolated nodes
NodeUpdate.forward calls scatter_sum(e2, i, dim=0) without dim_size.
When radius_graph omits certain node indices (no edges within cutoff),
i.max()+1 < num_nodes, causing downstream scatter_sum(v, batch, dim=0)
to fail with dimension mismatch. Fix by passing dim_size=num_nodes to
all NodeUpdate scatter_sum calls.
* fix PutAlongAxisGradNode NaN: replace put_along_axis with one-hot+matmul
_scatter_sum used paddle.put_along_axis(..., reduce='add') which has
a buggy backward kernel (PutAlongAxisGradNode) in Paddle 3.0, causing
NaN loss. Replaced dim=0 path with one-hot + matmul, which has a clean
well-defined gradient.
* fix NaN: disable create_graph=True for force grad
Paddle 3.0's Atan2GradNode and PutAlongAxisGradNode backward kernels
produce NaN when computing second-order gradients (create_graph=True).
Using create_graph=False avoids the buggy backward path while still
producing correct force predictions for evaluation.
* FIXME comments + detach angle/torsion atan2 for numerical stability
* Split val/test sections, add predict command to SphereNet README
* remove data symlink accidentally committed
* fix dataset log format to match existing PaddleMaterials style
* fix download log: remove misleading 'pretrained model' from generic download message
* fix qm9 _build_merged_index: handle multi-molecule xyz files + skip InChI line
* fix OOM: per-node triplet/quadruplet grouping in xyz_to_dat; reduce QM9 batch_size 128→32
* perf: precompute triplet/quadruplet indices in graph cache to eliminate O(∑ deg×deg) Python loop in xyz_to_dat
- Added compute_triplet_indices() to xyz_utils.py: extracts triplet
(idx_kj/idx_ji) and quadruplet (idx_lk/idx_triplet) indices from
edge_index alone, returning a dict of 1-D int64 tensors.
- xyz_to_dat() now accepts an optional precomputed_indices dict; when
provided, the Python per-node grouping loop is skipped entirely.
- QM9Dataset graph building step now calls compute_triplet_indices and
caches the result as a dict alongside edge_index in each .pkl file.
Backward-compatible: old .pkl (numpy array) detected via isinstance().
- SphereNet.forward / SphereNetPP.forward thread triplet_indices from
the data dict through to xyz_to_dat, converting numpy cache values
to tensors as needed.
Training step time expected: ~25s → <1s per batch.
* perf: also cache triplet/quadruplet indices in MD17Dataset graph building
* perf: use numpy arrays in compute_triplet_indices to avoid GPU sync per-element (10x graph building speedup)
* perf: use Paddle DataLoader with 24 workers for parallel graph building
Replace single-threaded loop with a DataLoader-based parallel
preprocessing pattern (similar to DiffSinger's approach). The
build step creates a lightweight _GraphBuildDataset whose
__getitem__ builds one graph + precomputes triplet/quadruplet
indices and saves to pkl in the worker process.
- QM9: _GraphBuildDataset reads molecules from merged xyz
- MD17: _MD17GraphBuildDataset reads positions from all_pos
- Both: DataLoader(num_workers=24, batch_size=1, use_shared_memory=False)
- Each worker creates its own converter instance (thread-safe)
- Results saved to disk inside worker; only idx returned to main
* fix: set paddle device to cpu in DataLoader workers (fork-safety)
GPU context is not fork-safe — worker processes that inherit GPU
context crash when running Paddle operations. Adding
paddle.set_device('cpu') at the start of each _GraphBuildDataset
and _MD17GraphBuildDataset.__getitem__ ensures workers use CPU
only, avoiding CUDA fork-unsafe errors.
* fix: use mp.Pool with spawn context + initializer for fork-safe parallel graph building
DataLoader with fork workers caused silent crashes (~35/133885
items processed) due to unsafe GPU context inheritance. Replaced
with multiprocessing.Pool using spawn context (fresh workers,
no GPU context). Module-level globals set via initializer(),
and per-item work dispatched via imap_unordered with tqdm.
QM9: _init_qm9_worker + _build_qm9_graph_idx
MD17: _init_md17_worker + _build_md17_graph_idx
* fix: use ThreadPoolExecutor instead of mp.Pool/mp.spawn for parallel graph building
- ThreadPoolExecutor avoids CUDA fork-safety issues (no GPU context inherited)
- Threads share memory, no pickling needed for worker functions
- Most operations release GIL (file I/O, numpy, Paddle CPU ops)
- Both QM9 and MD17 use 24 threads
* fix: handle triplet_indices in SphereNetCollator (batch offset)
triplet_indices is a nested dict of per-molecule edge/triplet-indexing
arrays. The collator now:
1. Excludes triplet_indices from node_keys/target_keys
2. Offsets edge-referencing keys (idx_kj, idx_ji, idx_lk, i, j)
by cumulative edge count per batch
3. Offsets triplet-referencing key (idx_triplet) by cumulative
triplet count
* fix: correct batch offsets in triplet_indices collation
i/j are NODE indices (not edge indices) — must be offset by
cumulative node count, not edge count. Previous code offset i/j
by edge count, causing GPU out-of-bounds access.
Correct offset strategy:
- i, j: node count offsets (same as edge_index)
- idx_kj, idx_ji, idx_lk: edge count offsets
- idx_triplet: triplet count offsets
* fix: reduce QM9 batch_size 32→8 for V100 16GB compatibility
A800 verification at bs=32 uses ~60GB GPU memory. V100 16GB
requires smaller batch. bs=8 estimated ~15GB on A800, safely
under V100 16GB limit.
* fix: reduce MD17 batch_size to 8 for V100 16GB
* feat: register 20 SphereNet pretrained weights in MODEL_REGISTRY
QM9 (12): mu, alpha, homo, lumo, gap, r2, zpve, U0, U, H, G, Cv
MD17 (8): aspirin, benzene_old, ethanol, malonaldehyde, naphthalene,
salicylic, toluene, uracil
All URLs point to reviewer-uploaded BOS checkpoints.
build_model_from_name('spherenet_qm9_mu') now auto-downloads weights.
* fix: robust zip structure handling + Predict section fallback
build_model_from_name: avoid double-joining model_name when zip
already contains the inner directory; use recursive os.walk to
find yaml config regardless of nesting depth.
PropertyPredictor: handle missing Predict section in config
(weights zips may contain only training config).
* feat: add SphereNet.predict() + build_model_from_name zip compat
- SphereNetPP.predict(): inference interface accepting data dicts
(z/pos/batch/edge_index). CIF/crystal support explicitly excluded
with clear error message — SphereNet is a molecular model.
- build_model_from_name: detect when zip already contains model_name
inner directory; use recursive os.walk as yaml fallback for any
nesting depth.
- PropertyPredictor: handle missing Predict section in config.
* feat(predict): add XYZ molecular input support
- New --xyz_file_path argument for molecular property prediction
- from_xyz_file() method parses XYZ, builds radius graph, predicts
- Example molecule qm9_sample.xyz (18 atoms)
- --cif_file_path no longer has a default (must be explicitly set)
to avoid accidentally running CIF prediction with molecular models
- Clear error when neither --xyz nor --cif is provided
* fix(predict): default to example XYZ when no input path given
Allows 'predict.py --model_name spherenet_md17_aspirin' to work
without specifying --xyz_file_path or --cif_file_path.
* revert: restore predict.py to original (reviewer requested no modifications)
Added example XYZ molecule (example_data/molecules/qm9_sample.xyz)
without changing predict.py code.
* fix: predict.py None check for config without Predict section + SphereNet Structure support
- PropertyPredictor: handle missing Predict config section (None check)
- SphereNetPP.predict(): accept pymatgen.Structure (auto-convert to
molecular data dict via species→Z + cart_coords→pos)
- predict.py otherwise unchanged (reviewer requested no modifications)
* feat: add molecule support to PropertyPredictor (from_xyz_file + --xyz_file_path)
Minimal changes to support molecular XYZ input alongside existing
CIF path:
- _SYMBOL_TO_Z module-level dict (QM9/MD17 element subset)
- from_xyz_file(): parses XYZ, builds graph converter from config,
predicts, supports single/directory multi-file + CSV save
- --xyz_file_path arg, dispatch in main (default stays CIF for
backward compat)
- Existing CIF flow completely unchanged
* refactor: split from_xyz_file into from_molecule + from_xyz_file
- from_molecule(atomic_numbers, positions, edge_index=None):
core prediction logic, accepts raw molecular data directly
- from_xyz_file: pure I/O layer, parses XYZ files and delegates
to from_molecule
* refactor: reuse graph_converter_fn in from_molecule
Move build_graph_cfg fallback from from_molecule into __init__
so graph_converter_fn is populated once at construction time.
from_molecule now simply calls self.graph_converter_fn(...) if
the converter is available — no inline config search.
* feat: add BuildMolecule pipeline to from_molecule (molecule_format param)
from_molecule now supports two modes:
1. Raw atomic data (z, pos, edge_index) — existing behavior unchanged
2. BuildMolecule pipeline: input → BuildMolecule → graph_converter → model, activated by passing molecule_format='smiles'|'rdmol'|'mol_file'|etc.
Follows the standard PaddleMaterials data pipeline: build_molecule → build_graph → predict
* feat: single BuildMolecule pipeline + --molecule+--format CLI
* fix: handle SMILES molecules without 3D conformer in from_molecule
mol.GetConformer() raises ValueError when no conformer exists
(e.g. from SMILES). Use try/except and generate 3D coordinates
via RDKit EmbedMolecule + MMFFOptimize
* refactor: simplify SphereNetCollator — drop auto-detection, keep concat core only
* refactor: align MD17Dataset cache with MP20 pattern (cache_path/overwrite/cfg check)
* refactor: MD17Dataset read_data method + remove random split logic
* refactor: align QM9 cache with MP20 pattern (is_equal check + naming)
* fix: remove on-the-fly split logic — use pre-split npz/indices files
* fix: address md17 review — STATS to class docstring, remove name validation, align rebuild with MP20, consolidate data storage
* chore: clean up md17 module docstring
* fix: remove tqdm fallback in qm9_dataset (align with md17)
* fix: replace lazy import in RadiusGraph with top-level import
* refactor: move radius_graph to datasets/graph_utils/spherenet_graph_utils, clean docstring
* fix: use math.pi instead of hardcoded pi, add SphereNet-specific annotation
* fix: xyz_utils docstring/import cleanup + fix use_torsion condition (idx_kj→idx_lk)
* refactor: encapsulate torsion selection as _select_min_abs_torsion helper
* fix: exclude k==i loopback triplets in compute_triplet_indices
* fix: remove _MOLECULE_URLS + individual URL fallback (bcebos only)
* fix: revert unintended DensityCollator formatting changes
* 精简 xyz_to_dat & SphereNet forward: 删除 precomputed_indices 过度抽象
- xyz_to_dat: 删除 precomputed_indices 参数, 内部直接调 compute_triplet_indices
消除双路径代码重复, 保证 k==i 排除和 _select_min_abs_torsion 一致性
- spherenet.py: 删除 forward() 的 triplet_indices 参数和预计算提取逻辑
- collate_fn.py: 还原 DensityCollator 无关注释/格式化改动(pre-commit 造成)
- datasets/__init__.py: 还原 pre-commit 删的 trailing space
* Round 16: 删除 (MP20 pattern) 等代码历史注释
* Round 16: force_key 参数补充文档说明
* Round 16: 删除 AngleEmbedding/TorsionEmbedding 未使用的 envelope_exponent (DIG残留)
* Round 16: md17 docstring 改为数据集介绍，去掉实现细节
* Round 16: SphereNetCollator 去掉 paddle tensor 转换，保持 numpy
* Round 16: SphereNetCollator 重组为 node/graph/edge/triplet 分块 + 改名 MolecularGraphCollator
* Round 16: md17 read_data 拆为 _download_raw + _ensure_splits + read_data 三步
* Round 16: qm9 删除 (MP20 pattern) 注释
* 按照评审员修改了逻辑
* 再次修改了评审员要求的内容
* 再次修改了评审员要求的内容
* 再次修改了评审员要求的内容
* 按评审意见修改
* Update and rename qm9_sample.xyz to isoguvacine.xyz
* CLI: remove --molecule/--format, add --xyz_file_path following --cif_file_path pattern
* RadiusGraph: support RDKit Mol input for predict.py pipeline, consistent with crystal Structure convention
* from_molecule: pass RDKit Mol to graph_converter_fn instead of (pos, batch)
* fix: restore --cif_file_path default path
* BuildMolecule移到顶部
* 删dataset中to_tensor
* 删raw路径和splits路径
* read_data改为MP20接口
* docstring对齐water格式
* 加overwrite和filter_unvalid属性
* 加energy_key参数
* 搬build_md17_graph到spherenet_graph_utils
* 简化下载逻辑
* split加str类型标注
* read_data改单参加索引
* cache逻辑改为MP20行内模式
* 变量名对齐MP20
* README示例改xyz_file_path
* eval_with_no_grad恢复上游写法
* post_transforms 不用多此一举
---------
Co-authored-by: leeleolay <805351297@qq.com>
|
Thanks for your contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.