Improving RDF

[PythonFZ]

I attempted to optimize the RDF code a little bit.

On an 108000 atoms system using

experiment.run_computation.RadialDistributionFunction(
    number_of_configurations=40, minibatch=32, batches=1, benchmark=True, correct_minibatch_batching=20
)

on an RTX2080 this code is about 20 % faster then the old code (0.76 GHz v. 0.91 GHz).

Main changes are:

• optimized TF function usage
• reduce redundancy
• optimize batch size so the GPU usage keeps almost constant over the full computation and not only on the start

Additions:

• benchmark kwarg
• correct_minibatch_batching kwarg

Still missing

• fix for large values on correct_minibatch_batching
• documentation
• small systems seem to fail

[SamTov]

Can you elaborate on the small systems seem to fail?

[PythonFZ]

Can you elaborate on the small systems seem to fail?

There are value errors and the RDFs look sometimes different to what they should look like.

[SamTov]

If you have any example that would be helpful. It's an odd problem.

[SamTov]

Is this PR ready to go?

[SamTov]

all the code looks good. if tests pass we can go with it unless there are some specific issues

[PythonFZ]

Unfortunately there are still issues with the Code that need to be resolved.

[SamTov]

Which ones? I'd like to bring this PR in so that I can run a black restructure on the code and merge the correctly formatted version. This could also wait until after all current PRs are done but it will make it a bit messier.

[SamTov]

Removing approval to avoid accidental merge.

[PythonFZ]

If I remember correctly, running it with different parameters for correct_minibatch_batching gave different results

[SamTov]

I will look into it tonight

[SamTov]

I've run the RDF on a 1000 atom system with differing correct mini batch arguments and received the same values each time. I will now check the small system size comment.

[SamTov]

@PythonFZ I think this can really be closed now.

[PythonFZ]

@PythonFZ I think this can really be closed now.

I don't think it has to - the idea is still valid and would improve the RDF speed. It needs to be fixed and tested before merging, but I don't see any need to close this PR.