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

[Bug]: pr-›orig_gdq is NULL. #219

Closed
1 task done
ktcrotts opened this issue Sep 27, 2024 · 14 comments · Fixed by #221
Closed
1 task done

[Bug]: pr-›orig_gdq is NULL. #219

ktcrotts opened this issue Sep 27, 2024 · 14 comments · Fixed by #221
Assignees
Labels
bug Something isn't working NIRCam Issue affects NIRCam data

Comments

@ktcrotts
Copy link

Check Existing Issues

  • Yes, I have checked existing issues to ensure this problem hasn't already been reported.

Instrument or Category

NIRCam Stage 1/2 Pipeline

Description

When running coron1pipeline.run_obs(), the error "pr-›orig_gdq is NULL." occurs unless "'algorithm': 'OLS'" is added under 'ramp_fit'.

orig_gdq

Error traceback output

No response

What operating system are you using?

Mac OS 14.7 Sonoma

What version of Python are you running?

Python 3.11

What Python packages do you have installed?

No response

Additional context or information

No response

@ktcrotts ktcrotts added the bug Something isn't working label Sep 27, 2024
@github-actions github-actions bot added the NIRCam Issue affects NIRCam data label Sep 27, 2024
@AarynnCarter
Copy link
Collaborator

@JarronL @mperrin Pretty sure this is something to do with the C implementation of the OLS fitting method.

Obviously we could make OLS the default rather than OLS_C, but it seems a shame to lose out on the faster fitting for longer ramp data. Any thoughts on what specifically we're doing with spaceKLIP that's affecting things?

@kglidic
Copy link
Collaborator

kglidic commented Oct 2, 2024

@ktcrotts Could you let us know what version of the jwst pipeline you were running with?

@kglidic kglidic self-assigned this Oct 2, 2024
@mperrin
Copy link
Collaborator

mperrin commented Oct 2, 2024

I can confirm this works fine for me (no error) on pipeline 1.15.1, with no special handling or parameter set for the ramp fit algorithm. i.e. using the default which is OLS_C now.
In my case the conda env is installed via stenv, not via spaceKLIP's requirements.txt.

@ktcrotts
Copy link
Author

ktcrotts commented Oct 2, 2024

@ktcrotts Could you let us know what version of the jwst pipeline you were running with?

I just looked and it says it's version 2.1.1.

@mperrin
Copy link
Collaborator

mperrin commented Oct 2, 2024

Hmm, that number doesn’t make sense. We would expect jwst.__version__ to be some value like “1.14.0” or “1.15.1” or “1.16.0”, but 1.16 is the current most recent.

@ktcrotts
Copy link
Author

ktcrotts commented Oct 3, 2024

Oh sorry for the confusion I was looking at the version of SpaceKLIP. The version for jwst is 1.16.0.

@mperrin
Copy link
Collaborator

mperrin commented Oct 3, 2024

OK, thanks. That's very helpful: I just confirmed that this works on 1.15.1 but errors on 1.16.0. In the same conda environment so the only differences are the jwst version plus its stcal dependency (1.7.3 and 1.9.0, respectively)

@mperrin
Copy link
Collaborator

mperrin commented Oct 3, 2024

I'll followup with the pipeline developers to try to figure out what's up and how to fix it.

@mperrin
Copy link
Collaborator

mperrin commented Oct 3, 2024

OK, this turns out to be a known issue (see spacetelescope/jwst#8842 and spacetelescope/jwst#8710) with the specific combination of: the OLS_C algorithm, multiprocessing, and possibly also the saveopt=True option.

There's a bug fix in this PR spacetelescope/stcal#289 which will go into the next version of stcal.

In the short term the workaround is to avoid multiprocessing with the OLS_C implementation. Specifically in the list of parameters for coron1pipeline, replace 'maximum_cores': "all" with 'maximum_cores': "1", for now. I've just confirmed that does work with 1.16.0 in my testing here.

FYI @AarynnCarter @strampelligiovanni @ktcrotts

@mperrin
Copy link
Collaborator

mperrin commented Oct 3, 2024

Oh and FYI @kglidic, see my comment above. We probably want to, at least temporarily, put a note in the tutorial notebooks about needing to change this parameter with 1.16.0.

Note, it's not enough to leave out the maximum_cores parameter. I tried that, but it turns out there's a line of code in spaceKLIP that sets the default to 'quarter' if no value is provided for maximum_cores. This is an override for the default behavior in the jwst pipeline itself, for which the default is simple 1 (no multiprocessing). So, for now it's necessary to explicitly set the value to 1 when calling coron1pipeline.run_obs.

@JarronL
Copy link
Collaborator

JarronL commented Oct 3, 2024

I addressed this in a Slack message in the coronagraphy-pipeline channel a couple weeks ago. Set maximum_cores='none' during run_obs will solve the problem (has the same effect as Marshall's solution).

image

I suggest we submit a PR with this as the default in SpaceKLIP.

e.g.,:

    # Number of processor cores to use during ramp fitting process
    # 'none', 'quarter', 'half', or 'all'
    pipeline.ramp_fit.maximum_cores      = kwargs.get('maximum_cores', 'none')

Replace this line:
https://github.com/kammerje/spaceKLIP/blob/6fa39c5f2afcc766bd66c0b667d81a21898c63b4/spaceKLIP/coron1pipeline.py#L717

@mperrin
Copy link
Collaborator

mperrin commented Oct 3, 2024

@ktcrotts might you be willing to make a pull request for this with the suggestion above from @JarronL, please ? Simple edit for any of us to do; I'm mostly suggesting you simply as a tiny practice exercise now in PR'ing code changes into this repo for you, if you'd like.

@mperrin
Copy link
Collaborator

mperrin commented Oct 3, 2024

@ktcrotts never mind, Jarron's too fast 😆

@JarronL
Copy link
Collaborator

JarronL commented Oct 3, 2024

@ktcrotts never mind, Jarron's too fast 😆

Oops!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working NIRCam Issue affects NIRCam data
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants