Skip to content
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.

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

Computation power spectra with folding #218

Open
JegerBroxterman opened this issue Dec 11, 2024 · 3 comments
Open

Computation power spectra with folding #218

JegerBroxterman opened this issue Dec 11, 2024 · 3 comments

Comments

@JegerBroxterman
Copy link

Hi,

The way the power spectra with folding are computed currently is by changing the positions of particles according to

box_over_fold_x = box_x / fold
inv_box_over_fold_x = 1.0 / box_over_fold_x
x_pos = (x_pos % box_over_fold_x) * inv_box_over_fold_x

with fold = 2.0^folding, i.e. (from swiftsimio readthedocs)
image

Should this not be
x_pos = (x_pos % box_over_fold_x) * fold ?

For example, without folding (folding = 0 and fold =1) the positions are now converted to
x_pos = (x_pos % (Lbox/1) ) * 1 / Lbox = x_pos / Lbox

@kyleaoman
Copy link
Member

At the very least the units seem wrong as-is but correct with the proposed change... I'd be happy to write a patch but I'm a little bit out of my depth on the theory here. @JBorrow if you can confirm that this is a bug as reported, I can probably fix it.

@JBorrow
Copy link
Member

JBorrow commented Feb 4, 2025

Why is this bad? Do you not want x_i' to be dimensionless bounded between 0 and 1? Perhaps the re-use of the variable is not ideal :)

@JegerBroxterman
Copy link
Author

I think the re-use of the same variable indeed is the thing that confused me. I think the current version of deposit() in the power_spectrum_py code works, but the descriptions of the input parameters are

    x : np.array[float64]
        array of x-positions of the particles. Must be bounded by [0, 1].

    box_x: float64
        box size in x, in the same rescaled length units as x, y, and z. c

whereas the input in render_to_deposit() is

    positions = data.coordinates

    # Get the box size
    box_size = data.metadata.boxsize

    # Deposit the particles
    arguments = dict(
        x=positions[:, 0].v,
        y=positions[:, 1].v,
        z=positions[:, 2].v,
        m=quantity.v,
        res=resolution,
        fold=folding,
        box_x=box_size[0].v,
        box_y=box_size[1].v,
        box_z=box_size[2].v,
    )

i.e. both box size and positions are not rescaled to [0,1]. I don't think it is an issue as both are not rescaled, but maybe change the description of the parameters in deposit() and deposit_parallel()?

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

No branches or pull requests

3 participants