Skip to content

Conversation

@mmolari
Copy link

@mmolari mmolari commented Nov 11, 2025

The convolution function had signature:

fn convolve(
    &self,
    input_grid: &Array1<f64>,
    f_values: &Array1<f64>,
    g_values: &Array1<f64>,
    output_grid: &Array1<f64>,
  ) -> Result<Array1<f64>, Report>;

But in most cases convolutions are implemented on grids with the same uniform spacing.
Moreover (except for the Riemann case) the convolution is always evaluated on the full output domain and then interpolated to fit the output grid

I think that in most cases it might be useful to simplify the signature of the convolve function to

fn convolve(&self, dx: f64, f_values: &Array1<f64>, g_values: &Array1<f64>) -> Result<Array1<f64>, Report>;

so that it takes as input simply:

  • the grid spacing dx
  • the two arrays with the values of the two functions on the grid
    It will then simply return the array of values of the convolution, on a grid of size |f| + |g| - 1

The restriction to particular output ranges or changes in grid sizes can then be done outside of the function. For example now the output grid determination is done here, outside of the function.

In this PR I:

  • changed the signature of the convolve method, as specified above
  • adapted and simplified the three specific implementations
  • adapted the tests to this new signature

let dx_a = compute_uniform_spacing(a.t())?;
let dx_b = compute_uniform_spacing(b.t())?;
// Use max instead of min to prevent exponentially growing grid sizes
let dx = dx_a.max(dx_b);
Copy link
Author

Choose a reason for hiding this comment

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

I also changed this to use first the fine-grained dx (minimum between the two) to make the convolution more precise, and then later interpolate the result of the convolution on the coarse-grained grid (maximum dx between the two)

ivan-aksamentov and others added 5 commits November 13, 2025 14:46
- change the signature of the `convolve` method, simplifying the underlying logic. Input/output grid are not arguments anymore, and only the grid size is passed.
- adapting the testing to this new signature
@ivan-aksamentov ivan-aksamentov merged commit 7fdb264 into rust Nov 13, 2025
8 of 9 checks passed
@ivan-aksamentov ivan-aksamentov deleted the feat/convolution-check branch November 13, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants