Skip to content

Benchmark for the RMS module fail #3520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
jbarnoud opened this issue Feb 4, 2022 · 2 comments · May be fixed by #5006
Open

Benchmark for the RMS module fail #3520

jbarnoud opened this issue Feb 4, 2022 · 2 comments · May be fixed by #5006
Labels
benchmarks performance benchmarking with ASV Continuous Integration defect

Comments

@jbarnoud
Copy link
Contributor

jbarnoud commented Feb 4, 2022

Expected behavior

Benchmarks run without errors.

Actual behavior

The benchmarks for analysis.rms fail with the following exceptions:

For parameters: 100, [1.0, 0.5], False, False
Traceback (most recent call last):
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 1184, in main_run_server
 main_run(run_args)
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 1058, in main_run
 result = benchmark.do_run()
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 537, in do_run
 return self.run(*self._current_params)
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 627, in run
 samples, number = self.benchmark_timing(timer, min_repeat, max_repeat,
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 694, in benchmark_timing
 timing = timer.timeit(number)
File "/home/benchy/mambaforge/lib/python3.9/timeit.py", line 177, in timeit
 timing = self.inner(it, self.timer)
File "<timeit-src>", line 6, in inner
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 599, in <lambda>
 func = lambda: self.func(*param)
File "/home/benchy/mdanalysis/benchmarks/benchmarks/analysis/rms.py", line 46, in time_rmsd
 rms.rmsd(a=self.A,
File "/home/benchy/mambaforge/lib/python3.9/site-packages/MDAnalysis/analysis/rms.py", line 260, in rmsd
 raise ValueError('weights must have same length as a and b')
ValueError: weights must have same length as a and b

For parameters: 100, [1.0, 0.5], False, True
Traceback (most recent call last):
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 1184, in main_run_server
 main_run(run_args)
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 1058, in main_run
 result = benchmark.do_run()
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 537, in do_run
 return self.run(*self._current_params)
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 627, in run
 samples, number = self.benchmark_timing(timer, min_repeat, max_repeat,
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 694, in benchmark_timing
 timing = timer.timeit(number)
File "/home/benchy/mambaforge/lib/python3.9/timeit.py", line 177, in timeit
 timing = self.inner(it, self.timer)
File "<timeit-src>", line 6, in inner
File "/home/benchy/mambaforge/lib/python3.9/site-packages/asv/benchmark.py", line 599, in <lambda>
 func = lambda: self.func(*param)
File "/home/benchy/mdanalysis/benchmarks/benchmarks/analysis/rms.py", line 46, in time_rmsd
 rms.rmsd(a=self.A,
File "/home/benchy/mambaforge/lib/python3.9/site-packages/MDAnalysis/analysis/rms.py", line 255, in rmsd
 a = a - np.average(a, axis=0, weights=weights)
File "<__array_function__ internals>", line 180, in average
File "/home/benchy/mambaforge/lib/python3.9/site-packages/numpy/lib/function_base.py", line 515, in average
 raise ValueError(
ValueError: Length of weights not compatible with specified axis.
@AnirG
Copy link
Contributor

AnirG commented Feb 14, 2022

Hey @jbarnoud! Weighted RMSD calculations require weight values for all atoms defined in the list, the way it is implemented. But in the benchmarking test, the parameter of the weights list hasn't been defined correctly corresponding to the number of atoms (contains only 2 values).
wrong_weights_list
Suggestions:

  1. Don't benchmark with weighted RMSD, as functionally it doesn't affect the benchmark much.
  2. Use a random list that adds up to one which contains num_atoms elements for corresponding num_atoms.
    Please guide me as I'm new to this stuff. I can take up this issue.

@orbeckst
Copy link
Member

For

mdanalysis/benchmarks/benchmarks/analysis/rms.py

Lines 17 to 24 in 55fa72f

params = ([100, 500, 2000],
[None, [1.0, 0.5]],
[False, True],
[False, True])
param_names = ['num_atoms',
'weights',
'center',
'superposition']
I suggest we change weights to be either True or None. We then let setup()

mdanalysis/benchmarks/benchmarks/analysis/rms.py

Line 26 in 55fa72f

def setup(self, num_atoms, weights, center, superposition):
either set self.weights to None or, for weights==True, to the weights computed from the masses according to the relative weight formula in the rmsd() docs

self.weights = self.A.masses() / np.sum(self.A.masses())

Then the actual call to

mdanalysis/benchmarks/benchmarks/analysis/rms.py

Lines 46 to 50 in 55fa72f

rms.rmsd(a=self.A,
b=self.B,
weights=weights,
center=center,
superposition=superposition)
in time_rmsd() can be simply

        rms.rmsd(a=self.A,
                 b=self.B,
                 weights=self.weights,
                 center=center,
                 superposition=superposition)

@ooprathamm ooprathamm mentioned this issue Mar 12, 2023
Closed
4 tasks
@tanishy7777 tanishy7777 linked a pull request Mar 31, 2025 that will close this issue
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
benchmarks performance benchmarking with ASV Continuous Integration defect
Projects
None yet
Milestone
No milestone
3 participants
@orbeckst @jbarnoud @AnirG