Provide size_2d argument to extract_compute

[swyant]

For global style computes that return an array, this keyword allows you to pre-specify the size of the array and skip the extra extract_compute calls to get SIZE_ROWS and SIZE_COLS.

The context for this is that I've been developing an interface to the POD potential in LAMMPS (see this PR ), which involves extracting from the pod/global compute. This is a very expensive compute, and the current version of extract_compute repeats it two additional times to get the row and column size, which makes things much slower on my end. I know the exact size of this array in advance, so having this keyword argument allows me to greatly increase performance.

Obviously, this is a very unsafe option, and I've tried to indicate this in the documentation.

[Joroks]

Do you know what exactly causes the slowdown? From how I've understood the documentation, calling lammps_extract_compute multiple times during the same timestep should not cause the compute to run again. However, if this does trigger the compute multiple times providing the size manually does seem like a good idea, but we should be consinstent here and also allow the user to do this for TYPE_VECTOR as well.

Another approach we could take is to directly call 'lammps_extract_compute' instead of our wrapper to determine the size of the array, which would save us some time spend on repeated input validation and would also allow us to skip wrapping the pointers in a vector first:

ndata = (style == STYLE_ATOM) ?
        extract_setting(lmp, "nlocal") :
        unsafe_load(reinterpret(Ptr{Int32}, API.lammps_extract_compute(lmp, name, style, API.LMP_SIZE_ROWS)))

count = unsafe_load(reinterpret(Ptr{Int32}, API.lammps_extract_compute(lmp, name, style, API.LMP_SIZE_COLS)))

with the same idea for TYPE_VECTOR as well

[vchuravy]

This is a very expensive compute, and the current version of extract_compute repeats it two additional times to get the row and column size, which makes things much slower on my end.

That would be a bug on the LAMMPS side or the potential. The code we are calling is

https://github.com/lammps/lammps/blob/43fbdc2d9385715ac01f9218defc5beca0afc853/src/library.cpp#L2364-L2365

It looks like it is the potential responsibility to set the "computed this timestep flag" https://github.com/lammps/lammps/blob/43fbdc2d9385715ac01f9218defc5beca0afc853/src/ML-PACE/compute_pace.cpp#L165

Which POD does not do https://github.com/lammps/lammps/blob/43fbdc2d9385715ac01f9218defc5beca0afc853/src/ML-POD/compute_pod_global.cpp#L110

[swyant]

Ah I see, yea that does seem to be a bug in ML-POD, I will try to fix it there. I'll close this out since it doesn't seem worthwhile to provide such an unsafe option if it's not necessary.