New integrator, and add some metadata to integrators.py

[reubenharry]

Addresses #679

• We should be able to understand what the PR does from its title only;
• There is a high-level description of the changes;
• There are links to all the relevant issues, discussions and PRs;
• The branch is rebased on the latest main commit;
• Commit messages follow these guidelines;
• The code respects the current naming conventions;
• Docstrings follow the numpy style guide
• pre-commit is installed and configured on your machine, and you ran it before opening the PR;
• There are tests covering the changes;
• The doc is up-to-date;

[codecov]

Codecov Report

Attention: Patch coverage is 46.00000% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 97.79%. Comparing base (7cf4f9d) to head (06dd04d).
Report is 8 commits behind head on main.

❗ Current head 06dd04d differs from pull request most recent head abe707c

Please upload reports for the commit abe707c to get more accurate results.

Files	Patch %	Lines
blackjax/mcmc/integrators.py	46.00%	27 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
- Coverage   98.87%   97.79%   -1.09%     
==========================================
  Files          59       59              
  Lines        2745     2806      +61     
==========================================
+ Hits         2714     2744      +30     
- Misses         31       62      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[reubenharry]

I see that code coverage is complaining about integrator_order, name_integrator and calls_per_integrator_step. These seem useful, but depending on your view, I could remove them

[junpenglao]

I see that code coverage is complaining about integrator_order, name_integrator and calls_per_integrator_step. These seem useful, but depending on your view, I could remove them

Yeah please remove them, depending on what you want to check there should be better ways to return these properties.

[junpenglao]

Remove? Are you planning to use it outside of integrators?

[reubenharry]

Yes, I have found it convenient to use them outside integrators. For example, if I want to run scripts that try different integrators, and I want access to their number of gradient calls. I suppose the other option would be to have a dictionary like {"velocity_verlet": {"num_grads": ..., "name": ..., "order": ...} }. Is that what you'd recommend?

As another example, I often want to do benchmarks against different integrators, but want to just iterate over X_coefficients, and then use generate_isokinetic_integrator for MCLMC and generate_euclidean_integrator for HMC

[reubenharry]

Also, it seems like code duplication to have isokinetic_velocity_verlet, velocity_verlet, etc, when we could just use the coefficients with generate_isokinetic_integrator and generate_euclidean_integrator

[junpenglao]

Also, it seems like code duplication to have isokinetic_velocity_verlet, velocity_verlet, etc, when we could just use the coefficients with generate_isokinetic_integrator and generate_euclidean_integrator

The design choice is motivated by how we are envisioning the usage of integrators in the library. Currently the design is to have integrator being a static set.

As another example, I often want to do benchmarks against different integrators, but want to just iterate over X_coefficients, and then use generate_isokinetic_integrator for MCLMC and generate_euclidean_integrator for HMC

You should iterate through the integrator objects instead

For example, if I want to run scripts that try different integrators, and I want access to their number of gradient calls. I suppose the other option would be to have a dictionary like {"velocity_verlet": {"num_grads": ..., "name": ..., "order": ...} }. Is that what you'd recommend?

Yes given that these are static, you should put them in your script as static parameters. I dont yet see those are useful in the library outside of benchmarking.

[reubenharry]

OK, that makes sense. I think my one point of disagreement is that if I don't expose the coefficients, nothing in the code "knows" that velocity_verlet and isokinetic_velocity_verlet are related. So I will have to have a dictionary of {"euclidean": velocity_verlet, "isokinetic": isokinetic_velocity_verlet, ...} when I want to compare each integrator on hmc vs mclmc, which I'm currently doing. This is a little painful, but not the end of the world

[reubenharry]

Updated, as per request