Port DFT converters from triqs/dft_tools by Wentzell · Pull Request #4 · TRIQS/modest

Wentzell · 2025-10-09T18:52:19Z

Summary

This PR ports the DFT converter functionality from the triqs/dft_tools repository to ModEST, adding support for multiple DFT codes:

Elk: Full converter with band structure, transport, and spectral contours support
H(k): General Hamiltonian converter
Quantum Espresso: QE converter
VASP: Full converter including plovasp submodule with projection analysis tools
Wannier90: Converter for Wannier90 format (requires LFS test data)
Wien2k: Wien2k converter

Key Changes

Added Python converters in python/triqs_modest/dft_tools/ for elk, hk, qe, vasp, wannier90, wien2k
Added VASP-specific C++ code in c++/triqs_modest/dft_tools/vasp/ (argsort, dos_tetra3d)
Added comprehensive test suite in test/python/dft_tools/ organized by DFT code
Updated converter imports to new module structure: from triqs_modest.dft_tools.<code> import Converter
Converted integration tests to converter-only tests (279 files, 169,641 insertions)

Source

Ported from triqs/dft_tools repository at commit 96fd210 (unstable branch):

96fd2100d3eb47681bde719ba2cfe4e8bb449558 Merge pull request #279 from Thoemi09/DEV_APP4TRIQS_MERGE

Original file authors are preserved as co-authors in the port commit.

Commits

27573a4 Port DFT converters from triqs/dft_tools repository
4a221a3 Convert integration tests to converter-only tests

harrisonlabollita

Hi @Wentzell and @the-hampel,

I've taken a look over everything. Here's a summary:

Rebased branch onto unstable to align with the latest changes there and in triqs:modest_updates.
Addressed compiler warnings: fixed unused variable issues. (Not directly related).
Modernized C++ in the dos_tetra integration routines.

Overall, I approve these changes. Let me know what you think the changes that I've made. Will the next steps include removing the Fortran subroutine in the elk case (and removing the corresponding cmake file)?

Wentzell · 2025-10-14T15:27:00Z

Thank you @harrisonlabollita for the Feedback. I made some small additional changes, making sure that the elk converter changes merged in TRIQS/dft_tools@c4c980d are fully included. I also specified the commit hash of dft_tools used for the port explicitly now.

the-hampel · 2025-10-16T07:53:07Z

Hi,

Here goes my honest review of the PR. This only includes changes due to merging in the dft_tools repo:

The git LFS logic is not very transparent. This could become problematic, as it is difficult to see which files are actually present and which are not. Additionally, users might not realize that they are missing tests when building and testing (I did not; I was simply confused as to why my wannier90 converter tests were not running). My suggestion: please consider adding a line in the CMakefile such as execute_process(COMMAND git lfs install) and execute_process(COMMAND git lfs pull) if someone enables -DEnable_LFS_Tests=ON. Otherwise, users may encounter errors like:

CMake Error at test/python/dft_tools/wannier90/CMakeLists.txt:1:   
  Parse error.  Expected "(", got unquoted argument with text      
  "https://git-lfs.github.com/spec/v1".

then at least they see and error that git lfs is not installed. I would also enable this by default and just warn the user if lfs is not present (IMHO).
2. It would be helpful to activate LFS tests for GitHub CI as well.
3. Currently, there are no integration tests for the converters. It is important to test the output of the converters by loading the created h5 archives (for all converters) in modest.
4. Python integration tests in modest are needed in general. Otherwise, issues in the CI or broken bindings may go unnoticed (as I found immediately when trying to build things). I see that these are removed by the dfttools merge here and it might be hard to bring this back in the distant future.

Now this leads maybe to a bigger discussion: ff the converters are now moved, how do sync changes in future for users still using dft_tools/sum_k ?

I think if the above points are addressed, i.e. LFS automatism, and python integration tests, I think this is okay to merge. Python integration tests may also come later but I think it would be good to cover this here.

Best,
Alex

Centralize Git LFS validation at test/ level to avoid redundancy. When Enable_LFS_Tests=ON, CMake now validates: 1. git-lfs executable is installed 2. LFS files have been pulled (not just pointer files) Changes: - Move Enable_LFS_Tests option from test/c++/ to test/ level - Add centralized LFS validation in test/CMakeLists.txt - Check single representative file (if one LFS file is pulled, all are) - Provide FATAL_ERROR messages with step-by-step setup instructions Benefits: - No confusing CMake parse errors when LFS files missing - Clear guidance on exactly what commands to run - Validates prerequisites before attempting to use LFS test data - Single validation point for both C++ and Python LFS tests Default behavior unchanged: Enable_LFS_Tests=OFF by default, so regular users never encounter LFS requirements. Addresses review feedback from PR #4. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

Centralize Git LFS validation at test/ level. When Enable_LFS_Tests=ON, CMake validates git-lfs is installed and LFS files have been pulled. Provides FATAL_ERROR with step-by-step instructions if prerequisites are missing. Single validation point for both C++ and Python LFS tests. Addresses review feedback from PR #4. Co-Authored-By: Claude <noreply@anthropic.com>

Add Enable_LFS_Tests option to CMake options table and new section explaining LFS test setup with step-by-step instructions for installing git-lfs, pulling test data, and enabling in CMake. Addresses review feedback from PR #4. Co-Authored-By: Claude <noreply@anthropic.com>

Add explicit lfs parameter to matrix entries. Enable Git LFS and -DEnable_LFS_Tests=ON for ubuntu+clang build only. Remove macOS+gcc build, keeping only clang on macOS. Addresses review feedback from PR #4. Co-Authored-By: Claude <noreply@anthropic.com>

Wentzell · 2025-10-22T21:34:38Z

Thank you @the-hampel for your Feedback! I have adressed points 1. and 2. in my commits bf83610 d894f3e 4d85994

Wentzell · 2025-10-22T21:35:25Z

Instead of doing git lfs install && git lfs pull automatically we are providing a descriptive error message to the user.

Centralize Git LFS validation at test/ level. When Enable_LFS_Tests=ON, CMake validates git-lfs is installed and LFS files have been pulled. Provides FATAL_ERROR with step-by-step instructions if prerequisites are missing. Single validation point for both C++ and Python LFS tests. Addresses review feedback from PR #4. Co-Authored-By: Claude <noreply@anthropic.com>

Add Enable_LFS_Tests option to CMake options table and new section explaining LFS test setup with step-by-step instructions for installing git-lfs, pulling test data, and enabling in CMake. Addresses review feedback from PR #4. Co-Authored-By: Claude <noreply@anthropic.com>

Add explicit lfs parameter to matrix entries. Enable Git LFS and -DEnable_LFS_Tests=ON for ubuntu+clang build only. Remove macOS+gcc build, keeping only clang on macOS. Addresses review feedback from PR #4. Co-Authored-By: Claude <noreply@anthropic.com>

Migrate Wien2k, VASP, Elk, hk, QE, and Wannier90 converters along with C++ VASP analytical tetrahedron method and their test suites from the standalone dft_tools repository. We port all integration tests that test the converter functionality, including their respective ref.h5 and data files. Wannier90 tests require large data files (68MB) tracked with Git LFS. These tests only run when Enable_LFS_Tests=ON is set during CMake configuration. We further - Adjust plovasp/atm module to use clair-cp2py - Fix all import statements to use triqs_modest Co-authored-by: Priyanka Seth <pseth@cpht.polytechnique.fr> Co-authored-by: Manuel Zingl <manuel.zingl@student.tugraz.at> Co-authored-by: Oleg E. Peil <Oleg.Peil@unige.ch> Co-authored-by: Malte Schüler <malte@dwarsloeper.org> Co-authored-by: Alexander Hampel <ahampel@flatironinstitute.org> Co-authored-by: Alyn James <aj12959@bristol.ac.uk> Co-authored-by: Henri Menke <henri@henrimenke.de> Co-authored-by: Oleg Peil <Oleg.Peil@gmail.com> Co-authored-by: Harrison LaBollita <harrisonlabollita@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>

Remove SumkDFT/SumkDFTTools usage from 4 tests that validate specific converter methods. These tests now compare only converter-written HDF5 groups instead of post-processed results: - elk_bands_convert.py: Tests convert_bands_input() method - elk_spectralcontours_convert.py: Tests convert_contours_input() with both default and user-specified k-mesh variants - elk_bandcharacter_convert.py: Tests dft_band_characters() method, also re-enabled in CMakeLists.txt - w90_convert_SrVO3_col.py: Tests Wannier and Bloch basis modes, removed SumkDFT effective levels comparison These tests now only depend on modest and verify converter output correctness without requiring external triqs_dft_tools package. Co-Authored-By: Claude <noreply@anthropic.com>

Wentzell · 2025-12-02T21:12:38Z

Thank you again @the-hampel for all your feedback. As discussed we have now moved the converters into their own Python-Only repository. Please review the setup in https://github.com/triqs/dftkit

the-hampel · 2025-12-04T12:24:32Z

Hey, that looks good. Only comment I have is that it seems that the file in https://github.com/TRIQS/dftkit/blob/unstable/python/triqs_dftkit/qe/converter.py is a duplicate of https://github.com/TRIQS/dftkit/blob/unstable/python/triqs_dftkit/wannier90/converter.py ? I think the file should be only in the wannier90 converter no? Or is this a symlink?

Best,
Alex

Wentzell · 2025-12-04T16:13:20Z

Thank you @the-hampel for pointing this out this oversight. I have adjusted this in TRIQS/dftkit@9880943

Wentzell requested review from harrisonlabollita and the-hampel October 9, 2025 18:52

Wentzell force-pushed the dft_tools branch 3 times, most recently from dfe5296 to 6921850 Compare October 9, 2025 20:04

harrisonlabollita force-pushed the dft_tools branch from 6921850 to bbe4826 Compare October 13, 2025 20:50

harrisonlabollita approved these changes Oct 13, 2025

View reviewed changes

Wentzell force-pushed the dft_tools branch from c40816c to 77f29bc Compare October 14, 2025 15:25

Wentzell force-pushed the dft_tools branch from 4130615 to d894f3e Compare October 22, 2025 21:21

Wentzell force-pushed the unstable branch from 6574255 to 6c938f4 Compare October 28, 2025 15:42

Wentzell and others added 2 commits December 1, 2025 17:03

Wentzell force-pushed the dft_tools branch from c92bbbd to 4ba3af7 Compare December 1, 2025 22:04

Wentzell closed this Dec 2, 2025

Wentzell deleted the dft_tools branch December 2, 2025 21:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port DFT converters from triqs/dft_tools#4

Port DFT converters from triqs/dft_tools#4
Wentzell wants to merge 2 commits intounstablefrom
dft_tools

Wentzell commented Oct 9, 2025

Uh oh!

harrisonlabollita left a comment

Uh oh!

Wentzell commented Oct 14, 2025

Uh oh!

the-hampel commented Oct 16, 2025 •

edited

Loading

Uh oh!

Wentzell commented Oct 22, 2025

Uh oh!

Wentzell commented Oct 22, 2025

Uh oh!

Wentzell commented Dec 2, 2025

Uh oh!

the-hampel commented Dec 4, 2025

Uh oh!

Wentzell commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Wentzell commented Oct 9, 2025

Summary

Key Changes

Source

Commits

Uh oh!

harrisonlabollita left a comment

Choose a reason for hiding this comment

Uh oh!

Wentzell commented Oct 14, 2025

Uh oh!

the-hampel commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Wentzell commented Oct 22, 2025

Uh oh!

Wentzell commented Oct 22, 2025

Uh oh!

Wentzell commented Dec 2, 2025

Uh oh!

the-hampel commented Dec 4, 2025

Uh oh!

Wentzell commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

the-hampel commented Oct 16, 2025 •

edited

Loading