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

Fixed bary_corr issue in and refactor of extract.py to remove repeated code #943

Merged
merged 1 commit into from Apr 3, 2020

Conversation

akremin
Copy link
Member

@akremin akremin commented Apr 3, 2020

Addressed issue #941 . Also goes ahead and refactors extract.py to remove repeated code.

Tested using different numbers of tasks and cpus-per-task ( srun ... -n -c ).

Tested with night 20200314, exposure 55445.

Barycentric correction is applied appropriately now. Below I show examples in cam=z1.

Normal -n 20 -c 2:
image

Using -n 2 -c 32:
image

No mpi:
image

The two mpi versions have bitwise-identical data.
The non-mpi version does differ to a minor extent, I believe this is due to the precision loss in the read/write operations in the mpi version which writes temp files before consolidation.

Below I show the sum of all differences in a given HDU (over all values in the HDU.data).

b1
FLUX -0.012568751
IVAR 9.1393304e-08
RESOLUTION 2.3883758e-05
CHI2PIX 0.0015733168

r1
FLUX 0.004627799
IVAR 7.579832e-07
RESOLUTION 1.715213e-05
CHI2PIX -0.000250794

z1
FLUX -0.046244144
IVAR -3.4323136e-08
RESOLUTION 1.4052641e-05
CHI2PIX 0.0011709929

@akremin akremin requested a review from sbailey April 3, 2020 19:13
@akremin akremin added this to In progress in Data Release 20.4 via automation Apr 3, 2020
@akremin
Copy link
Member Author

akremin commented Apr 3, 2020

I showed the straight sum of differenced pixels, here I show the more useful sum of the abs for all pixels:

Results are unsurprisingly higher:
b1
FLUX 2.7046037
IVAR 1.382468e-05
RESOLUTION 0.0042085135
CHI2PIX 0.036702715

r1
FLUX 4.6476836
IVAR 1.9532863e-05
RESOLUTION 0.007019518
CHI2PIX 0.036098786

z1
FLUX 22.699293
IVAR 3.2151926e-05
RESOLUTION 0.0072929785
CHI2PIX 0.07679814

but keep in mind this is a sum over 500 * 2881 = 1440500 differences.

@sbailey
Copy link
Contributor

sbailey commented Apr 3, 2020

I'm still a bit mystified about why there are differences at all, but I agree that they are very small. I also checked the flux difference normalized by the errors, and the max differences are 0.004 sigma. I also ran just 2 bundles and got identical fluxes. So it's a bit odd, but I think this is good enough to merge.

Side note: there is a bug in the MPI extractions that predates this PR, where it writes out 500 spectra regardless of the --nspec parameter. Laurie, Mark, and I are working on another extraction refactor in prep for GPUs so we can leave this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants