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

Add lags to vafs #318

Merged
merged 4 commits into from
Nov 4, 2024
Merged

Add lags to vafs #318

merged 4 commits into from
Nov 4, 2024

Conversation

oerc0122
Copy link
Collaborator

@oerc0122 oerc0122 commented Oct 2, 2024

Fixes #253

@oerc0122 oerc0122 added the enhancement New/improved feature or request label Oct 2, 2024
@oerc0122 oerc0122 self-assigned this Oct 2, 2024
@oerc0122 oerc0122 force-pushed the 253-vaf-lags branch 2 times, most recently from 4a0a989 to 484c2d3 Compare October 2, 2024 14:19
@ElliottKasoar
Copy link
Member

Are these lags consistent with the on-the-fly correlations (@harveydevereux)?

@harveydevereux
Copy link
Collaborator

Are these lags consistent with the on-the-fly correlations (@harveydevereux)?

Yes these will be, given the right on-the-fly options. But it is possible to have correlations returned with lags that don't have a uniform step due to the window averaging, sample frequency options etc.

lags = np.arange(n_steps) * time_step

Currently don't have fft as an option though, perhaps we should

@oerc0122 oerc0122 force-pushed the 253-vaf-lags branch 2 times, most recently from da7363d to 18010a4 Compare October 14, 2024 10:41
Copy link
Member

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

I think test_post_process.py::test_vaf needs updating to get the vaf from the returned tuple, and ideally probably should check the lag while it's at it too.

janus_core/helpers/post_process.py Outdated Show resolved Hide resolved
janus_core/helpers/post_process.py Outdated Show resolved Hide resolved
@oerc0122 oerc0122 force-pushed the 253-vaf-lags branch 2 times, most recently from 64e2ec1 to ce710bb Compare November 4, 2024 11:28
Copy link
Member

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the notes on filter_atoms!

@ElliottKasoar ElliottKasoar merged commit 8678f94 into main Nov 4, 2024
16 checks passed
@ElliottKasoar ElliottKasoar deleted the 253-vaf-lags branch November 4, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New/improved feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

post-process issues
3 participants