Skip to content

[MRG] Add QSW sampling for sliced Wasserstein - #838

Open
Samuel-Vangu wants to merge 7 commits into
PythonOT:masterfrom
Samuel-Vangu:feature/add-qsw-sampling
Open

[MRG] Add QSW sampling for sliced Wasserstein#838
Samuel-Vangu wants to merge 7 commits into
PythonOT:masterfrom
Samuel-Vangu:feature/add-qsw-sampling

Conversation

@Samuel-Vangu

Copy link
Copy Markdown

Types of changes

  • New feature
  • Documentation update
  • Tests

Motivation and context / Related issue

Closes #835

This PR adds Quasi-Monte Carlo (QMC) sampling of projection directions to the Sliced Wasserstein module.

Currently, sliced_wasserstein_distance samples projection directions uniformly at random, corresponding to standard Monte Carlo sampling. This PR adds two alternatives based on the generalized spiral point construction described in [Nguyen, Bariletto & Ho (2024)](https://arxiv.org/abs/2309.11713):

  • sampling_slices="qsw": deterministic Quasi-Sliced Wasserstein (QSW) projection directions.
  • sampling_slices="rqsw": Randomized QSW (RQSW), obtained by applying a random rotation to the deterministic spiral point set.

The new sampling methods are currently limited to 3D, while the existing "uniform" sampling remains the default.

The implementation also exposes get_projections_spiral and updates the documentation, README references, release notes, and adds a 3D example.

How has this been tested (if it applies)

The changes have been tested with:

  • pre-commit run --all-files — all checks pass.

  • pytest test/sliced/test_sliced_distances.py62 tests passed.

  • Added tests covering:

    • deterministic spiral projections;
    • randomized projections and sphere preservation;
    • seed reproducibility;
    • invalid dimensions and sampling methods;
    • QSW/RQSW with NumPy, JAX, PyTorch and TensorFlow backends;
    • consistency of deterministic QSW across backends;
    • QSW approximation compared with uniform Monte Carlo sampling in 3D.

PR checklist

@Samuel-Vangu

Copy link
Copy Markdown
Author

Hi @rflamary, @clbonet,

This one is ready for your review whenever you have time. Thanks!

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.71429% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.84%. Comparing base (9439b8a) to head (90f62f3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #838      +/-   ##
==========================================
- Coverage   96.86%   96.84%   -0.03%     
==========================================
  Files         128      128              
  Lines       25947    26119     +172     
==========================================
+ Hits        25133    25294     +161     
- Misses        814      825      +11     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@clbonet clbonet self-assigned this Aug 28, 2026

@clbonet clbonet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Samuel-Vangu for the great PR! Overall the code is great.

About the name of the method in "sampling_slices", I am wondering whether we should put something more precised than "qsw". The generalized_spiral seems to be the fastest method, but maybe we would like to add other Quasi Monte-Carlo methods in the future such as the minimization of the Coulomb energy or something else. Thus, I think we should put something more precised, e.g. "spiral_qmc"?

Also, for the randomized option. Since it can be applied to any Quasi-Monte Carlo method, maybe it should be "randomized_spiral_qmc", and we can get the boolean with checking "randomized" in sampling_slices?

What do you think @Samuel-Vangu, @rflamary ?

Otherwise, I have few minor comments below.


By default, the projection directions :math:`\theta` are sampled uniformly
at random. Setting ``sampling_slices`` to ``"qsw"`` or ``"rqsw"`` instead
uses Quasi-Monte Carlo point sets on the sphere (generalized spiral

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the generalized spiral the best QMC method? With the current API, we can't really add other QMC methods such as Equal area mappings, Coulomb energy etc...

Would it be worth it to add this in the names?

Comment thread ot/sliced/_utils.py Outdated
):
r"""
Generates n_projections points on the sphere via generalized
spiral points (Rakhmanov, Saff & Zhou, 1994).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the ref

Comment thread ot/sliced/_utils.py


def get_projections_spiral(
d, n_projections, randomized=True, seed=None, backend=None, type_as=None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generalized spirals are only valid for d==3, so maybe we don't need d here

Comment thread ot/sliced/_utils.py
Comment on lines +282 to +285
if d != 3:
raise ValueError(
f"get_projections_spiral is only implemented for d=3, got d={d}"
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed?

Comment thread ot/sliced/_sliced_distances.py Outdated
Comment on lines +150 to +157
elif sampling_slices == "qsw":
projections = get_projections_spiral(
d, n_projections, randomized=False, backend=nx, type_as=X_s
)
elif sampling_slices == "rqsw":
projections = get_projections_spiral(
d, n_projections, randomized=True, seed=seed, backend=nx, type_as=X_s
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put "randomized_qsw" in the name, and then use a variable randomized="randomized" in sampling_slices. It would be better if we want to add more QMC sampling methods later

Comment thread ot/sliced/_utils.py Outdated
Comment on lines +292 to +294
# sin/cos are not exposed by the backend abstraction (only arccos, atan2
# exist), so the deterministic point construction is done in plain NumPy
# and converted to the target backend at the end.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add sin and cos to the backend and use nx instead of np

Comment thread RELEASES.md Outdated
Comment on lines +2 to +5
## 0.9.8dev
*August 2026*

#### New features

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To delete

Comment thread RELEASES.md Outdated
Comment on lines +7 to +9
- Add Quasi-Monte Carlo sliced Wasserstein sampling (QSW/RQSW) via generalized
spiral points, selectable with `sampling_slices` in `sliced_wasserstein_distance`,
as described in [93] (PR #838)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To move below

between two point clouds -- known here in closed form, with no
approximation error left except from the number of projections itself.

.. [93] Nguyen, K., Bariletto, N., & Ho, N. (2024). Quasi-Monte Carlo for

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update [93] to [95]

@Samuel-Vangu

Copy link
Copy Markdown
Author

Hi @clbonet,

Thanks for the detailed review! Here's a summary of what I changed in response to each point:

Naming (sampling_slices values)
Agreed — "qsw"/"rqsw" were too generic given that spiral is just one of several possible QMC constructions (equal-area, Coulomb energy, etc. could come later). Renamed to "spiral_qmc" and "randomized_spiral_qmc", and the dispatcher now derives the boolean via sampling_slices.startswith("randomized_") plus a method = sampling_slices.removeprefix("randomized_"), so adding a new QMC construction later only needs one new elif, not two.

Missing reference
Added Rakhmanov, Saff & Zhou (1994) as [96] in the README, and cited it in get_projections_spiral's docstring alongside [95] (Nguyen et al.) which is now correctly numbered.

d parameter
Kept it in the signature — for consistency with get_projections_sphere and so future d-generic constructions (Coulomb, equal-area) fit the same dispatch pattern — but added if d != 3: raise ValueError(...) inside get_projections_spiral itself, not just in the dispatcher. That way a user calling the function directly (not through sliced_wasserstein_distance) is still protected, not only callers going through sampling_slices.

sin/cos on the backend
Added sin/cos to Backend and all five implementations. I kept get_projections_spiral's deterministic point construction in plain NumPy though, rather than switching to nx: most backends' arange() doesn't propagate type_as's dtype (e.g. JAX defaults to float32 without x64 enabled), so building natively per-backend would likely break test_qsw_matches_across_backends's exact cross-backend equality check.

RELEASES.md
Removed the duplicate ## 0.9.8dev header and merged the entry under the existing section.

[93][95]
Fixed in plot_qsw_3d.py and everywhere else it appeared.

Let me know if any of these choices don't sit right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Quasi-Monte Carlo point sets for the Sliced Wasserstein module

3 participants