msg-byu / symlib

Spacegroup finder. Includes symmetry-related routines for cluster expansion and other codes that rely on symmetries of lattices and crystals.
MIT License
12 stars 15 forks source link

find_value_in_array issues. #5

Closed wsmorgan closed 9 years ago

wsmorgan commented 9 years ago

Each of the subroutines in this interface take 4 arguments, an array, a value or value list, an output location array, and an output integer for the number of times the value array appeared. The current issue is that the output location array has to be allocated outside the subroutines so that we have to "guess at the size before hand" if we fail to do that properly then we get results like this:

4          6         7       805306368         2133590033         2133596230       2133596267       2133596304

where the only the first 3 elements of the output array have meaning. The rest are of course random garbage from the machine. For unit tests I can get around this by pre-assigning the loc vector to have values of zero. The question is do we want to change this so that the only values returned are those that have meaning?

rosenbrockc commented 9 years ago

In looking at the routine, the following are obvious:

  1. loc can never have a dimension that exceeds that of array.
  2. nfound is being returned so that the user knows how many of the values in loc to use when calling an array splice: array(loc(1:nfound)) would return the only the values in vals.

The construction of find_lvalue_in_larray1 is a little confusing since the conditional any(array(l).eqv.vals(:)) has a logical scalar array(l) on the LHS of the .eqv. but an array vals(:) on the RHS (hence the use of any). However, logical types can only ever be T or F, so this only makes sense if all the values in vals were the same; otherwise it would always return true because no matter what array(l) is, it would be equal to any of the elements (since they include both T and F) unless vals is always an array with only one element. In any case, the conditional isn't transparent.

Since nfound is returned, it doesn't matter that the rest of the entries are garbage. The alternative is to have an allocatable, intent(out) for loc, but that will necessitate changing the code whenever that interface is called, so probably just leave it as it is.