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

Fix noise analysis for "narrowband" data with no ECORR #45

Merged
merged 2 commits into from
Jul 25, 2023

Conversation

rossjjennings
Copy link
Member

In trying to run the process_v1.2 notebook on simulated LOFAR data, @FrancescoIraci ran into an issue (#44) where noise_utils.add_noise_to_model() would raise an exception if "narrowband" data didn't have associated ECORR parameters.

It turns out that this function was written in an unnecessarily inflexible way: with using_wideband = False, it would always try to add ECORR parameters to the model, whether or not they were present in the white noise dictionary returned by noise_utils.analyze_noise(). In this case, not finding any, it would raise an exception. (Similarly, with using_wideband = True, it would always try to add DMEFAC and DMEQUAD parameters, even if none were present.)

This PR changes this behavior to make it more flexible: now, ECORR, DMEFAC, and DMEQUAD parameters should be added if, and only if, they are present in the white noise dictionary, regardless of the value of using_wideband, which now only serves to identify the appropriate noise chain directory. This should make it possible to run the notebook on narrowband data without EFAC, or on wideband data without DMEFAC and/or DMEQUAD.

@rossjjennings
Copy link
Member Author

rossjjennings commented Jul 21, 2023

Hmm. These test failures are confusing. It looks like it's nothing to do with this PR really -- it's failing to create an environment because the available versions of enterprise_extensions don't support Python 3.11, and everything (even the "3.8" test) is trying to use 3.11!

@rossjjennings
Copy link
Member Author

It looks like a GitHub Action thing. @JPGlaser any ideas?

@tcromartie tcromartie merged commit edff929 into nanograv:main Jul 25, 2023
3 checks passed
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.

3 participants