Skip to content

[Codescan] Grow the basin attractor table instead of relying on a fixed 5 percent heuristic #36

Description

@Stardust0831

Global scan provenance

This issue was found by the Codex global repository scan of non-unit-test files in Stardust0831/Multiwfn, pinned to commit d2b770a531eb7039153198d04520b5992f15c49d.

Problem

generatebasin() allocates the attractor coordinate table with capacity nint(nx*ny*nz/20D0), assuming attractors can never exceed five percent of grid points.

Multiwfn/basin.f90

Lines 860 to 870 in d2b770a

use basinintmod
implicit real*8 (a-h,o-z)
integer,parameter :: nmaxtrjgrid=3000
integer igridmethod
integer ntrjgrid !Recording how many members are contained in trjgrid now
integer trjgrid(3,nmaxtrjgrid) !The trajectory contains which grids, record their indices sequentially, trjgrid(1/2/3,i) = ix/iy/iz of the ith grid
! real*8 trjval(nmaxtrjgrid),gradmaxval(nmaxtrjgrid) !******For debugging******
if (allocated(attgrid)) deallocate(attgrid)
allocate(attgrid(3,nint(nx*ny*nz/20D0))) !I think the number of attractors in general is impossible to exceeds nx*ny*nz/20
numatt=0
write(*,*) " Attractor X,Y,Z coordinate (Angstrom) Value"

New attractors increment numatt and write attgrid(:,numatt) without checking that the heuristic capacity is nonzero or still sufficient:

Multiwfn/basin.f90

Lines 946 to 961 in d2b770a

end if
exit cyciatt
end if
end do
end do cyciatt
!Found a new attractor. Add to list, and assign the grids in the current trajectory
if (iatt==numatt+1) then
if (itime==1) then
numatt=numatt+1
do itrjgrid=1,ntrjgrid
gridbas(trjgrid(1,itrjgrid),trjgrid(2,itrjgrid),trjgrid(3,itrjgrid))=numatt
end do
attgrid(1,numatt)=inowx
attgrid(2,numatt)=inowy
attgrid(3,numatt)=inowz
call getgridxyz(inowx,inowy,inowz,tmpx,tmpy,tmpz)

The basin grid dimensions are user-configurable, so very small grids can round the allocation down to zero, while noisy or synthetic scalar fields can contain more local attractors than the undocumented five-percent assumption.

Multiwfn/basin.f90

Lines 1688 to 1705 in d2b770a

if (igridsel==4) dx=spclunaqual
if (igridsel==5) then
write(*,*) "Input the grid spacing (Bohr), e.g. 0.08"
read(*,*) dx
end if
dy=dx
dz=dx
nx=nint(molxlen/dx)+1
ny=nint(molylen/dy)+1
nz=nint(molzlen/dz)+1
else if (igridsel==6) then
write(*,*) "Input the number of grid points in X,Y,Z direction, e.g. 139,59,80"
read(*,*) nx,ny,nz
dx=molxlen/(nx-1)
dy=molylen/(ny-1)
dz=molzlen/(nz-1)
else if (igridsel==7) then
write(*,*) "Input X,Y,Z coordinate of original point (Bohr), e.g. 0.1,4,-1"

Impact

The first detected attractor on a zero-capacity table, or any attractor beyond the heuristic capacity, writes out of bounds. Bounds-checked builds terminate; optimized builds may corrupt adjacent memory and later produce incorrect basin assignments.

Suggested direction

  • Allocate a documented minimum capacity and grow attgrid geometrically when numatt reaches capacity.
  • Check multiplication and conversion for integer overflow before sizing from nx*ny*nz.
  • Alternatively collect attractors in a dynamically growing container and materialize the final array afterward.
  • Add tests for fewer than 20 grid points and for an adversarial field with more than five percent local attractors.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions