PyWiFeS / pipeline

The Python data reduction pipeline for WiFeS
6 stars 26 forks source link

Twilight flat scattered light #39

Closed clidman closed 2 months ago

clidman commented 6 months ago

It seems that the scattered light in the twilight flat is not computed.

CMartinezLombilla commented 6 months ago

Thank you, Chris! I'll check what's going on there.

CMartinezLombilla commented 3 months ago

Hi @clidman, I'm so sorry that it took me ages to come back to this issue. I found out what's going on and indeed it's something that has been happening all this time and we didn't realise so thank you so much for reporting. The issue is in the configuration files as in the flat_cleanup step, there is only one type of flats instead of both types, that is, type = ["dome", "twi"]. By doing just that, the scattered light in the twilight flat is computed.

However, I have a couple of questions about that:

  1. I did the test by just adding the ''twi'' type to the blue arm. Is there any situation (or grating) in which we need to do this correction also in the red arm?

  2. There are a few params below type in the flat_cleanup step, such as "offsets", "radius" or "nsig_lim". The data looks nicely corrected without changing anything in there but not sure if I'm missing something. Do you know if is there any parameter here that we should tweak?

  3. Could you please give it a try by editing your .json file in pipeline/reduction_scripts and let me know how it goes? You just need to set up type= ["dome", "twi"] in the flat_cleanup step for the blue arm and run the pipeline as always.

Hope this helps!

clidman commented 3 months ago

Hi Cristina, Some answers.

Cheers,

Chris

A/Professor Chris Lidman Director, Siding Spring Observatory, Research School of Astronomy and Astrophysics, The Australian National University, Canberra, ACT, Australia Work: +61-2-6125-0238 @.**@.>

From: Cristina Martinez-Lombilla @.> Date: Wednesday, 27 March 2024 at 10:12 am To: PyWiFeS/pipeline @.> Cc: Christopher Lidman @.>, Mention @.> Subject: Re: [PyWiFeS/pipeline] Twilight flat scattered light (Issue #39)

Hi @clidmanhttps://github.com/clidman, I'm so sorry that it took me ages to come back to this issue. I found out what's going on and indeed it's something that has been happening all this time and we didn't realise so thank you so much for reporting. The issue is in the configuration files as in the flat_cleanup step, there is only one type of flats instead of both types, that is, type = ["dome", "twi"]. By doing just that, the scattered light in the twilight flat is computed.

However, I have a couple of questions about that:

  1. I did the test by just adding the ''twi'' type to the blue arm. Is there any situation (or grating) in which we need to do this correction also in the red arm?

It is needed for both arms. If a twilight flat is unavailable, then processing the data with just the deom flat is fine.

  1. There are a few params below type in the flat_cleanup step, such as "offsets", "radius" or "nsig_lim". The data looks nicely corrected without changing anything in there but not sure if I'm missing something. Do you know if is there any parameter here that we should tweak? Not that I am aware of.

For the blue arm, I use 'offsets':[0.4,0.4], 'radius':10.0, 'nsig_lim':3.0}

For the red arm, I use

'offsets':[0.4,0.4], ‘buffer’:4; 'radius':10.0, 'nsig_lim':3.0}

                            I suggest that you use these values to start with.
  1. Could you please give it a try by editing your .json file in pipeline/reduction_scripts and let me know how it goes? You just need to set up type= ["dome", "twi"] in the flat_cleanup step for the blue arm and run the pipeline as always.

I’ll try this tomorrow or Monday next week

Hope this helps!

— Reply to this email directly, view it on GitHubhttps://github.com/PyWiFeS/pipeline/issues/39#issuecomment-2021620343, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADYGRBBRFGZTOEQY756E35TY2H6FNAVCNFSM6AAAAABAUKTGHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRRGYZDAMZUGM. You are receiving this because you were mentioned.Message ID: @.***>

CMartinezLombilla commented 3 months ago

Awesome! Thank you, @clidman! I'll update the setup with the configuration that you suggest. I'd recommend then that you also add the ''twi'' type to the red arm so the correction is performed in both arms. I'll try it myself too.

No worries about testing it quickly, I'll be out until next Friday. I'll keep you posted on the outcomes of my tests.

Cheers, Cristina.

CMartinezLombilla commented 3 months ago

Hi @clidman, could you test this? If all goes ok, I'll apply the changes in the pipeline this week. Thank you!

clidman commented 3 months ago

I tried to test this, but I am no longer able to install the pipeline.

Running pip install . I get

Processing /Users/clidman/Science/Programs/PyWiFeS_Auto/pipeline Preparing metadata (setup.py) ... done Requirement already satisfied: wheel in /Users/clidman/opt/anaconda3/envs/py3p12/lib/python3.12/site-packages (from pywifes==0.7.4) (0.43.0) Requirement already satisfied: setuptools in /Users/clidman/opt/anaconda3/envs/py3p12/lib/python3.12/site-packages (from pywifes==0.7.4) (69.2.0) Collecting astropy (from pywifes==0.7.4) Using cached astropy-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl.metadata (9.8 kB) INFO: pip is looking at multiple versions of pywifes to determine which version is compatible with other requirements. This could take a while. ERROR: Ignored the following yanked versions: 1.11.0 ERROR: Ignored the following versions that require a different python version: 1.10.0 Requires-Python <3.12,>=3.8; 1.10.0rc1 Requires-Python <3.12,>=3.8; 1.10.0rc2 Requires-Python <3.12,>=3.8; 1.10.1 Requires-Python <3.12,>=3.8; 1.6.2 Requires-Python >=3.7,<3.10; 1.6.3 Requires-Python >=3.7,<3.10; 1.7.0 Requires-Python >=3.7,<3.10; 1.7.1 Requires-Python >=3.7,<3.10; 1.7.2 Requires-Python >=3.7,<3.11; 1.7.3 Requires-Python >=3.7,<3.11; 1.8.0 Requires-Python >=3.8,<3.11; 1.8.0rc1 Requires-Python >=3.8,<3.11; 1.8.0rc2 Requires-Python >=3.8,<3.11; 1.8.0rc3 Requires-Python >=3.8,<3.11; 1.8.0rc4 Requires-Python >=3.8,<3.11; 1.8.1 Requires-Python >=3.8,<3.11; 1.9.0 Requires-Python >=3.8,<3.12; 1.9.0rc1 Requires-Python >=3.8,<3.12; 1.9.0rc2 Requires-Python >=3.8,<3.12; 1.9.0rc3 Requires-Python >=3.8,<3.12; 1.9.1 Requires-Python >=3.8,<3.12 ERROR: Could not find a version that satisfies the requirement scipy==1.9.1 (from pywifes) (from versions: 0.8.0, 0.9.0, 0.10.0, 0.10.1, 0.11.0, 0.12.0, 0.12.1, 0.13.0, 0.13.1, 0.13.2, 0.13.3, 0.14.0, 0.14.1, 0.15.0, 0.15.1, 0.16.0, 0.16.1, 0.17.0, 0.17.1, 0.18.0, 0.18.1, 0.19.0, 0.19.1, 1.0.0, 1.0.1, 1.1.0, 1.2.0, 1.2.1, 1.2.2, 1.2.3, 1.3.0, 1.3.1, 1.3.2, 1.3.3, 1.4.0, 1.4.1, 1.5.0, 1.5.1, 1.5.2, 1.5.3, 1.5.4, 1.6.0, 1.6.1, 1.9.2, 1.9.3, 1.11.0rc1, 1.11.0rc2, 1.11.1, 1.11.2, 1.11.3, 1.11.4, 1.12.0rc1, 1.12.0rc2, 1.12.0, 1.13.0rc1, 1.13.0

I'll have a deeper look at why it is failing later today.

CMartinezLombilla commented 3 months ago

Thank you @clidman! I've been taking a look at what's going on and I think the problem is that we had to constrain some Python modules to certain versions but I think we should have restricted the Python version too as it should be lower than 3.12 (and I think this is the version you have). Please, try this: copy and paste this configuration below in your setup.py file (and remove the current code), then do pip install .. I think this should work (and in that case, I'll also fix it in the repo), so please, let me know how it goes.

from setuptools import find_packages, setup

setup(
    name="pywifes",
    version="0.7.4",
    packages=find_packages(where="src"),
    package_dir={"": "src"},
    python_requires=">=3.8, <3.12",  # Adjusted Python version range
    install_requires=[
        "wheel",
        "setuptools",
        "astropy",
        "numpy",
        "matplotlib",
        "photutils==1.8.0",
        "pandas",
        "scipy==1.9.1",  # SciPy version is fixed to 1.9.1
    ],
    description="A Python package for optical data reduction pipeline.",
)
clidman commented 3 months ago

Hi Cristina, In the end, I installed python 3.10 in its own conda environment

conda create --name PyWiFeS python=3.10 conda activate PyWiFeS pip install .

This worked for python 3.19, but failed with python 3.11. You may need to restrict the python version further.

It’s now running with 3.10. I’ll let you know how it goes.

Cheers,

Chris

A/Professor Chris Lidman Director, Siding Spring Observatory, Research School of Astronomy and Astrophysics, The Australian National University, Canberra, ACT, Australia Work: +61-2-6125-0238 @.**@.> [signature_1443578104]

From: Cristina Martinez-Lombilla @.> Date: Tuesday, 9 April 2024 at 11:29 am To: PyWiFeS/pipeline @.> Cc: Christopher Lidman @.>, Mention @.> Subject: Re: [PyWiFeS/pipeline] Twilight flat scattered light (Issue #39)

Thank you @clidmanhttps://github.com/clidman! I've been taking a look at what's going on and I think the problem is that we had to constrain some Python modules to certain versions but I think we should have restricted the Python version too as it should be lower than 3.12 (and I think this is the version you have). Please, try this: copy and paste this configuration below in your setup.py file (and remove the current code), then do pip install .. I think this should work (and in that case, I'll also fix it in the repo), so please, let me know how it goes.

from setuptools import find_packages, setup

setup(

name="pywifes",

version="0.7.4",

packages=find_packages(where="src"),

package_dir={"": "src"},

python_requires=">=3.8, <3.12",  # Adjusted Python version range

install_requires=[

    "wheel",

    "setuptools",

    "astropy",

    "numpy",

    "matplotlib",

    "photutils==1.8.0",

    "pandas",

    "scipy==1.9.1",  # SciPy version is fixed to 1.9.1

],

description="A Python package for optical data reduction pipeline.",

)

— Reply to this email directly, view it on GitHubhttps://github.com/PyWiFeS/pipeline/issues/39#issuecomment-2043983350, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADYGRBGS5SJ6TELGAQRUUX3Y4NAAXAVCNFSM6AAAAABAUKTGHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBTHE4DGMZVGA. You are receiving this because you were mentioned.Message ID: @.***>

clidman commented 3 months ago

Only the blue twilight flats are produced.

clidman commented 2 months ago

Hi Cristina, I had a closer look at the products that are produced.

I did not see the following calibration files for the red arm

wifes_red_super_twiflat.fits wifes_red_super_twiflat_mef.fits wifes_red_super_twiflat_raw_corr.fits wifes_red_super_twiflat_raw.fits

and the blue arm

wifes_blue_super_twiflat.fits wifes_blue_super_twiflat_raw_corr.fits

I looked at the json config files.

If there is a twilight flat, then

{'step': 'flat_cleanup', 'run': True, 'suffix': None, 'args': {'type': ['dome'], 'verbose': True, 'plot': False, 'offsets': [0.4, 0.4], 'radius': 10.0, 'nsig_lim': 3.0}}

should be

{'step': 'flat_cleanup', 'run': True, 'suffix': None, 'args': {'type': ['dome','twi'], 'verbose': True, 'plot': False, 'offsets': [0.4, 0.4], 'radius': 10.0, 'nsig_lim': 3.0}}

Do you have a list of which steps are done in which order.

CMartinezLombilla commented 2 months ago

Hi @clidman, yes I know about that configuration and I've been working on that, thank you! I have updated the json files so the data reduction applies the twilight flats correction in both red and blue arms. I have also updated the requirements to constrain the Python version to 3.10. The steps that now also consider the twilight flats in both arms are:

The steps are done in the order in which they are listed in the json files and in the current configuration, they are all running (param "run": true), except the ' wire_soln' in the red arm.

Should we also set the 'wire_soln" in red as 'true'? We just kept it as it was in the previous config files, but this is easy to change.

You can give a try to this new set up by running the pipeline from the branch cml_twi_flats. Please, let me know how it goes and if I need to make any change (in my tests the twilight flats are now also applied in the red arm).

If all goes ok, I'll implement these updates into the automation branch.

CMartinezLombilla commented 2 months ago

Hi @clidman! Did you have a chance to test this implementation? I'd need to merge these changes into automation asap so please, let me know if you're happy with the results now. Thank you!!

clidman commented 2 months ago

Sorry for the delayed reply.

I reran the pipeline on a test dataset. The twilight flat fields are created for both arms now and the extracted and spliced spectrum look fine. Good job.

I see no reason why you should not keep the wire solution in the red arm too.

CMartinezLombilla commented 2 months ago

Hi Chris, I've already applied these changes to the automation branch so the pipeline does make these corrections. Thank you!