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

modified post-processing code to be able to choose axis #107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andreufont
Copy link
Owner

@andreufont andreufont commented Nov 1, 2020

Adds option to specify the axis along which we want to extract the skewers and measure p1d.

It also includes book-keeping changes in extract_skewers.py and snapshot_admin.py related to issue #98.

When using axis=None, the code reproduces the current p1d (with the book-keeping changes mentioned above).

Before merging this, we should wait for GriddedSpectra in fake_spectra to be updated: sbird/fake_spectra#42

@andreufont andreufont self-assigned this Nov 1, 2020
@Chris-Pedersen
Copy link
Collaborator

Chris-Pedersen commented Jan 11, 2021

Hi @andreufont I'm happy to pick this up, have you tested it works with Simeon's master on fake_spectra? We could switch to using that instead of my modified fake_spectra branch. As far as I understand it temperature rescalings are possible on master, but I haven't experimented with using the RateNetwork class to do this, and we would also have to modify the existing scripts to work with the RateNetwork syntax.

The other thing that would change if we switch branches is the fit range. Currently when I compare the TDR fits between my and Simeon's branches, there's a ~4% difference, likely due to differences in the fit ranges and method.

So I guess we either:

Switch to master and:

  1. Test the RateNetwork temperature rescalings, and rework our scripts to use them
  2. Decide on whether to recalculate the T0 fits using Simeon's master, or modify the fit range to be consistent with our previous values

Stick with my modified branch and update it to include the multiple-axis commits that you added to fake_spectra master.

Either way I don't think we need to store the p1d along each axis as part of the emulator data, only the combined p1d.

Lets discuss this tomorrow!

@andreufont
Copy link
Owner Author

Yes, master branch in fake_spectra works for set_axis, I've used it extensively in Sherwood sims.

Good point about rescalings and fitting ranges, let's chat later today.

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