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

Implemeted MRI IFU empirical cruciform #928

Closed
wants to merge 1 commit into from

Conversation

patapisp
Copy link

@patapisp patapisp commented Nov 3, 2024

This is the the start of a PR for finalising the broadening and cruciform implementation for the MIRI IFU mode. Added an additional option for model_type = "cruciform"

Remaining tasks:

  • check normalisation convention when modifying PSF models
  • changing the oversample parameter yields quite significant differences in the PSF models
  • switch empirical broadening (now following Law+2023) to broadening due to sampling by detector and cube pixels

image

@pep8speaks
Copy link

Hello @patapisp, Thank you for submitting the Pull Request !

Line 609:64: E226 missing whitespace around arithmetic operator
Line 610:102: E226 missing whitespace around arithmetic operator
Line 611:106: E226 missing whitespace around arithmetic operator
Line 698:23: E226 missing whitespace around arithmetic operator
Line 741:21: E226 missing whitespace around arithmetic operator
Line 741:25: E226 missing whitespace around arithmetic operator

If you have not done so, please lint your code in accordance with best practices.

@mperrin
Copy link
Collaborator

mperrin commented Nov 8, 2024

I'm noting here for the record that the workflow on this one is going to be atypical. Rather than giving a code review to Polychronis and then having him revise the PR, I'm going to take over and be editing and finishing this one myself.

@mperrin
Copy link
Collaborator

mperrin commented Nov 8, 2024

See #930 instead

@mperrin mperrin closed this Dec 18, 2024
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.

3 participants