Support for Fastspec PCA templates #260
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Restored original transmission_Lyman - Moved the new calculations to transmission_IGM_old - Created new transmission_IGM that has vectorized and GPU-ized operations that match output of transmission_IGM_old For now I simply have the existing transmission_Lyman return either transmission_IGM or transmission_IGM_old, but eventually we will want to change the code to change calls to transmission_Lyman to transmission_IGM assuming this function will supplant the old one (or we can just rename it but I wanted to keep them distinct for now so as to not mess with other branches). Also eventually we may have logic where a grid is calculated once and then used to interpolate values instead of repeating the calculations every time. Note that transmission_IGM_old calls IGM.full_IGM_old - I have kept the functions that calls included for now for easy comparison with similar _old appended to them. These versions will go away as the vectorized version will work with 1 or N redshifts. Also note I added copy_data_to_gpu method which caches and resizes various data arrays read from disk. Finally note on lines 743-747 - I believe what we want to pass to full_IGM is lObs and not lRF because lRF already is divided by (1+z) - and this would not make sense when for instance doing comparisons like `x0 = lobs < lamL*(1.+zS)[:,None]` - I think the comparison should be between lObs and lamL*(1+zS) OR between lRF and lamL. We might want to eventually change the code to do the latter so that we get rid of the extra multiplication or even pass both lRF and lObs such that we don't have to do any extra multiplication or division. Also I believe that ` i = min_wave < 1215.` will give us the correct behavior we want instead of the hack of just looking at z < 2.
…edrock into fastspec-pca-templates Attempting merge after git reported conflict due to branch being modified by me and someone else
|
The new Inoue IGM code is not covered by unit tests yet; I will add that as part of a separate IGM refactor branch+PR. I have verified that this branch produces identical outputs as main when run with default parameters. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm opening this PR with changes from John and Craig to add the Inoue14 IGM model with GPU support, plus command line options to specify redshift scan ranges per template. However, to maintain compatibility with current templates and runs, I'm leaving the Calura12/Irsic13 IGM model as the default. In a separate branch+PR I'll refactor how IGM models are handled to support backwards compatibility of IGM models while allowing Inoue14 for future templates.