Skip to content

Fix: some easy-to-fix problems in [Code scan] issues#7582

Draft
Critsium-xy wants to merge 7 commits into
deepmodeling:developfrom
Critsium-xy:fix/code-scan-tier1
Draft

Fix: some easy-to-fix problems in [Code scan] issues#7582
Critsium-xy wants to merge 7 commits into
deepmodeling:developfrom
Critsium-xy:fix/code-scan-tier1

Conversation

@Critsium-xy

@Critsium-xy Critsium-xy commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes a batch of low-risk, localized issues reported by the recent Codex global repository scan (the "[Code scan]" issues). Each fix is a separate commit referencing its issue.

Fixes

Issue Fix
#7541 Correct syntax error (fdir suffix + s_dirfdir + s_dir) in the RT-TDDFT projection tool
#7542 Repair empty else branch (add : no-op) in generate_orbital_mixstru.sh so bash -n passes
#7552 Reject one-past angular momentum index in TwoCenterTable::is_present (l <=l <)
#7551 Copy the innermost dimension (size[2], not size[1]) when slicing 3D tensors
#7548 Delete OFDFT KEDF_Manager with scalar delete to match its allocation
#7572 Use the consistent SUPERLU_DIST32_ROOT variable in CI path exports
#7555 Read LATTICE_PARAMETER blocks consistently in the Multiwfn and pyabacus STRU parsers (partial)

Notes on scope:

Verification

  • Python files pass python -m py_compile.
  • The shell script passes bash -n.
  • C++ / CMake / CI changes are localized one-liners reviewed against surrounding idioms.

Critsium-xy and others added 5 commits July 2, 2026 14:34
Line 9 contained the invalid expression `fdir suffix + s_dir`, which
made projection.py unparseable. Use `fdir + s_dir` to match the working
example copy.

Closes deepmodeling#7541

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`bash -n` failed on the empty `else` branch before `fi`. Add a `:`
no-op so the example script is syntactically valid.

Closes deepmodeling#7542

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
index_map_ is allocated with final dimension length bra.lmax()+ket.lmax()+1,
so valid indices are 0..dim_size(6)-1. The bounds check used `l <=
dim_size(6)`, allowing a one-past read. Use `l < dim_size(6)` to match the
neighboring dimension checks.

Closes deepmodeling#7552

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each 3D row is placed at offset_out advancing by size[2], so the
contiguous copy length must also be size[2]. The copy used size[1],
corrupting non-cubic slices where size[1] != size[2].

Closes deepmodeling#7551

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kedf_manager_ is allocated with scalar `new KEDF_Manager()`, but the
reinitialization path in before_all_runners() freed it with `delete[]`,
which is undefined behavior. Use scalar `delete` to match the allocation.

Closes deepmodeling#7548

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Critsium-xy Critsium-xy changed the title Fix: batch of Tier-1 [Code scan] issues Fix: some easy-to-fix problems in [Code scan] issues Jul 2, 2026
@Growl1234

Growl1234 commented Jul 2, 2026

Copy link
Copy Markdown

I fully disagree with the fix regarding #7568 and #7572; the reason, also what the real problem should lie in, has already been described simply under those issues.

  • Adding DEFAULT_MSG would only make FindMKL.cmake more fragile.
  • We should try to drop redundant environment variables in CI workflows if possible rather than only fix the known issue.

@Critsium-xy

Copy link
Copy Markdown
Collaborator Author

I fully disagree with the fix regarding #7568 and #7572; the reason, also what the real problem should lie in, has already been described simply under those issues.

  • Adding DEFAULT_MSG would only make FindMKL.cmake more fragile.
  • We should try to drop redundant environment variables in CI workflows if possible rather than only fix the known issue.

Okay, I will further investigate about both issues and this PR. This is only a draft now.

@Growl1234

Copy link
Copy Markdown

I'm having a deeper look about CMake cleanup these days, and the revision about FindMKL.cmake (and SetupCuSolverMp.cmake) is on my to-do list. In order to avoid duplication, would you agree with I keeping an eye on these files under this PR?

@Critsium-xy

Copy link
Copy Markdown
Collaborator Author

I'm having a deeper look about CMake cleanup these days, and the revision about FindMKL.cmake (and SetupCuSolverMp.cmake) is on my to-do list. In order to avoid duplication, would you agree with I keeping an eye on these files under this PR?

As you are trying to fix the thing about FindMKL.cmake, I will remove the commits about this file in my PR.

@kirk0830

kirk0830 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

@Critsium-xy hi, because there are also commits to push from @QuantumMisaka side, I suggest pick the fix related with abacuslite out from this PR, in case of time consuming conflict-solving.
Never mind, the developers of abacuslite are still active ;)

LD_LIBRARY_PATH, PKG_CONFIG_PATH and CPATH referenced the misspelled
SUPERLU32_DIST_ROOT while CMAKE_PREFIX_PATH used SUPERLU_DIST32_ROOT (the
name defined in Dockerfile.intel). Align the path exports to
SUPERLU_DIST32_ROOT so SuperLU_DIST is not silently omitted.

Closes deepmodeling#7572

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Critsium-xy Critsium-xy force-pushed the fix/code-scan-tier1 branch from dc1f11a to 1c0d273 Compare July 3, 2026 02:14
@Critsium-xy

Copy link
Copy Markdown
Collaborator Author

@Critsium-xy hi, because there are also commits to push from @QuantumMisaka side, I suggest pick the fix related with abacuslite out from this PR, in case of time consuming conflict-solving. Never mind, the developers of abacuslite are still active ;)

Ok I will quickly do that.

The Multiwfn and pyabacus STRU parsers checked for a LATTICE_PARAMETER
block but then read blocks['LATTICE_PARAMETERS'] with an extra S, and
treated the list of lines as a string. Read
blocks['LATTICE_PARAMETER'][0].split() to match the neighboring
LATTICE_CONSTANT idiom, so STRU files using LATTICE_PARAMETER parse
instead of raising KeyError.

The identical fix for the ASE AbacusLite parser is intentionally left out
of this PR and will be handled separately.

Refs deepmodeling#7555

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Critsium-xy Critsium-xy force-pushed the fix/code-scan-tier1 branch from 1c0d273 to db5774d Compare July 3, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants