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

Renaming psd-related variables to avoid confusion #64

Merged
merged 1 commit into from
Jan 28, 2024

Conversation

tsunhopang
Copy link
Collaborator

The load_psd function is modified such that

  1. If no PSD file is provided, it automatically fetches the preset ASD files and squares them to get the PSD
  2. If a PSD file is provided, treat the values in the file as PSD without squaring

This is to avoid confusion with the current implementation of the load_psd function, as it treats the inputs as ASD files.

@tsunhopang
Copy link
Collaborator Author

tsunhopang commented Jan 28, 2024

I also suggest to move the load_psd away from inject_signal and have it run during the initialization.

@kazewong kazewong self-requested a review January 28, 2024 18:18
@kazewong
Copy link
Owner

Regarding moving the load_psd, I think it is more general to keep a separate method to load the actual data as it is now, since in the case of fetching real data, the PSD/ASD file is never fetched in the way load_psd is called.

The rest of the PR looks good to me, merging to main.

@kazewong kazewong merged commit 9ed508f into kazewong:main Jan 28, 2024
5 of 7 checks passed
@tsunhopang
Copy link
Collaborator Author

tsunhopang commented Jan 28, 2024

In production, we usually use BayesLine PSD as input rather than re-estimating the PSD using the Welch method. So the load_psd is actually more realistic for production run.

@tsunhopang tsunhopang deleted the load_psd_naming branch February 2, 2024 09:53
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