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

All BIAS files are processed by CALACS to use only read noise for ERR… #288

Merged

Conversation

Projects
None yet
3 participants
@mdlpstsci
Copy link
Contributor

commented Jan 8, 2018

All BIAS files are now processed by CALACS to use only read noise (vs Poisson) for the computation of the ERR array as the date discriminant (pre- and post-SM4) has been removed. Also, when using ACSREJ standalone, the user now must use the "readnoise_only" flag (formerly "newbias" flag) to obtain the same results as CALACS for BIAS images. NOTE: Modifying the flag to ACSREJ is expeditious rather than user-friendly. The "newbias" flag has been removed (not just deprecated) as this functionality is used mostly internally for calibration purposes. If the "newbias" flag is invoked, ACSREJ will issue an message regarding the change and exit with an error.

When updating ACSREJ for standalone use, improved the syntax or usage message which addresses a portion of #278 for ACSREJ only.

Resolves #277. (EDIT: Added a space for proper cross-linking.)
Addresses some components of #278 for ACSREJ only.

@mdlpstsci mdlpstsci self-assigned this Jan 8, 2018

@mdlpstsci mdlpstsci force-pushed the mdlpstsci:issue277-biasFrame-ERRData branch from c2be923 to 3fae3a0 Jan 8, 2018

@pllim
Copy link
Contributor

left a comment

Needs a corresponding PR over at ACSTOOLS. Otherwise, LGTM.

syntax_error ("acsrej input output [-t] [-v]");
printf(" [-shadcorr] [-crmask] [-newbias] \n ");
syntax_error ("acsrej.e input output [-t] [-v]");
printf(" [-shadcorr] [-crmask] [-readnoise_only] \n");

This comment has been minimized.

Copy link
@pllim

pllim Jan 9, 2018

Contributor

This needs a corresponding change over at ACSTOOLS (e.g., https://github.com/spacetelescope/acstools/blob/master/acstools/pars/acsrej.cfg).

This comment has been minimized.

Copy link
@jamienoss

jamienoss Jan 9, 2018

Member

My input: I would argue that this 1-1 API mapping creates an unnecessary dependency. I would state that it is better to use the, recently added, 'transparent window' exe_args and then just make this issue one of documentation only (since the ACS HSTCAL docs exist as doc strings in acstools). However, I understand that since the other parameters are mirrored the precedence is already set and it's already too late.

@jamienoss
Copy link
Member

left a comment

This looks good to me other than this one comment.

} else if (strcmp("newbias", argv[ctoken]+1) == 0) {
par->newbias = 1;
printf ("\n*************************************************************************************\n");
printf (" As of HSTCAL Build 2018.1, the 'newbias' option for ACSREJ has been renamed \n");

This comment has been minimized.

Copy link
@jamienoss

jamienoss Jan 9, 2018

Member

I'm not sure of our official stance on this but we may need to refer to HSTCAL by something other than our internal build identifier (2018.1), i.e. the actual version number. This definitely needs to be done, I'm just not sure if we need both or just the latter. It may even need to also reference the version of CALACS.

@pllim

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2018

Corresponding Python wrapper changes are in spacetelescope/acstools#51. FYI.

@mdlpstsci mdlpstsci force-pushed the mdlpstsci:issue277-biasFrame-ERRData branch from 368b657 to 016cfb4 Jan 11, 2018

All BIAS files are processed by CALACS to use only read noise for ERR…
… array

Fixes #277.

All BIAS files are processed by CALACS to use only read noise (vs Poisson)
for the computation of the ERR array as the date discriminant has been
removed. Also, when using ACSREJ standalone, the user now must use the
"readnoise_only" flag (formerly newbias) to obtain the same results as CALACS
for BIAS images.  When updating ACSREJ for standalone use, improved the
syntax or usage message which addresses a portion of #278 for ACSREJ only.

@mdlpstsci mdlpstsci force-pushed the mdlpstsci:issue277-biasFrame-ERRData branch from 016cfb4 to b6d7c1d Jan 12, 2018

@mdlpstsci mdlpstsci merged commit e1d4a30 into spacetelescope:master Jan 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.