ehpor / hcipy

A framework for performing optical propagation simulations, meant for high contrast imaging, in Python.
https://hcipy.org
MIT License
91 stars 30 forks source link

VLTI aperture #145

Closed syhaffert closed 1 year ago

syhaffert commented 2 years ago

An implementation of the VLTI aperture. Based on the ESO P105 VLTI manual.

codecov[bot] commented 2 years ago

Codecov Report

Merging #145 (62584ad) into master (8c29370) will decrease coverage by 0.22%. The diff coverage is 69.28%.

@@            Coverage Diff             @@
##           master     #145      +/-   ##
==========================================
- Coverage   80.93%   80.70%   -0.23%     
==========================================
  Files          95       95              
  Lines        6923     7044     +121     
==========================================
+ Hits         5603     5685      +82     
- Misses       1320     1359      +39     
Impacted Files Coverage Δ
hcipy/aperture/__init__.py 100.00% <ø> (ø)
hcipy/dev.py 100.00% <ø> (ø)
hcipy/optics/glass.py 84.78% <ø> (ø)
hcipy/aperture/generic.py 89.43% <14.28%> (-1.48%) :arrow_down:
hcipy/aperture/realistic.py 91.11% <64.76%> (-5.46%) :arrow_down:
hcipy/wavefront_sensing/pyramid.py 73.33% <100.00%> (+8.27%) :arrow_up:
hcipy/optics/optical_element.py 71.87% <0.00%> (ø)
hcipy/field/grid.py 81.86% <0.00%> (+0.54%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

syhaffert commented 2 years ago

An image of the telescope positions image

And the baselines image

ehpor commented 2 years ago
  • Put pngs of the reference/baseline apertures as a comment.

I meant tests/baseline_for_apertures/vlti/pupil_without_spiders.fits.gz and tests/baseline_for_apertures/vlti/pupil.fits.gz and the like. :) But these are great images for reference too!

syhaffert commented 2 years ago

I think the aperture will also change with azimuth. Think about two telescopes, what happens when you observe at the horizon and parallel to the telescopes, you will only see a single aperture.

Should I also add the dOPD to the apertures ? Because the dOPD is Baseline * direction vector.

ehpor commented 2 years ago

I think the aperture will also change with azimuth. Think about two telescopes, what happens when you observe at the horizon and parallel to the telescopes, you will only see a single aperture.

Which is why I said "pointing on the sky" rather than "altitude". 😛

Should I also add the dOPD to the apertures ? Because the dOPD is Baseline * direction vector.

Maybe have a separate function for that rather than integrate it into make_vlti_aperture()? Or at least have a separate function for calculating the dOPDs?

FYI, I would be fine with just having a few sentences in the docstring without any extra implementation for non-Zenith pointing, ie. leave it as it is now. Whichever you feel like doing. The failed tests need to be fixed though.

syhaffert commented 2 years ago

Ok. I can implement both next week. I have already done the coding for the different on-sky pointings. I was thinking you were only talking about altitude because you mentioned zenith. My brain just completely ignored azimuth at that point.

syhaffert commented 1 year ago

All changes have been implemented, tests have been added. All checks have been passed. I think this is ready to be merged.

ehpor commented 1 year ago

You still didn't post the baseline apertures. It makes reviewing much easier. I'll try to get the review in tomorrow or the day after.

The baseline apertures used for the tests: from left to right, top to bottom, pupil, pupil_non_zenith, pupil_non_zenith_without_spiders, pupil_without_spiders: image