PR: https://github.com/lsst-ts/ts_phosim/pull/50

As I noticed in the PR:

tests/test_skySim.py::testAddStarByChipPos was failing due to some issue with upstream packages. Either some change in the DM stack or ts_wep is causing the failure. I update the values so the test will pass, if we can fix this in the upstream I can remove the patch before merging.

I added conda recipe files but the build will not work unless we run it with --no-test. We still need to figure out how to make builds for packages that depend on the stack, but I thought it would be nice to have the recipe added here already. If you disagree I can remove them as well.

For the update of ts_phosim, we may need to check the simulation can run without the problem first. Krzysztof Suberlak can you help to check?

Added the hotfix/unittest branch in ts_phosim to fix the unit test. This failed is because the CCD dimension has been changed in the ts_wep, which used the real camera instead of the PhoSim's original camera dimension.

Removed this because Tiago has this fix in ticket branch already.

I've just run `imgCloseLoop`  yesterday on NCSA with  `ts_phosim` on master (1.3.1), `ts_wep` on master (1.6.2) , `phosim_utils` on master (0.2.4), and it converged. Te-Wei Tsai , do you mean to test the `skySim` functionality? 
When I run `imgCloseLoop` I provide the input sky file:

python bin.src/imgCloseLoop.py --inst comcam --numOfProc 25 --boresightDeg 0.03 -0.02 --skyFile tests/testData/sky/skyComCam.txt --output /project/scichris/aos/perturbations/imgCloseLoop_0-05/

So here skySim is used only to read the ra,dec from file  https://github.com/lsst-ts/ts_phosim/blob/e25ac907d0385b109caf1ee4a803d9f248b54096/python/lsst/ts/phosim/SkySim.py#L126-L154  

If I run it in that mode, I don't think it uses the SkySim functionality of converting ccd position to ra,dec , which is considered  in the failing test  (https://github.com/lsst-ts/ts_phosim/blob/e25ac907d0385b109caf1ee4a803d9f248b54096/tests/test_skySim.py#L125-L142 ) . So to test the conversion of ccd x,y to ra,dec , I could do something like in this notebook https://github.com/suberlak/AOS/blob/main/AOS_letter_simulate.ipynb   .  Te-Wei Tsai , is this what you had in mind ? 

By the way, looking deeper into the issue, I would have thought that this test https://github.com/lsst-ts/ts_phosim/blob/e25ac907d0385b109caf1ee4a803d9f248b54096/tests/test_opdMetrology.py#L214-L226 should also fail since it tests almost identical functionality (ccd x,y --> ra,dec). But upon inspection the difference is that 

• SkySim  in  _getSkyPosByChipPos https://github.com/lsst-ts/ts_phosim/blob/e25ac907d0385b109caf1ee4a803d9f248b54096/python/lsst/ts/phosim/SkySim.py#L348-L356   calls : 

  # Get the pixel positions in DM team

  # this function just does the X,Y --> Y,X

  pixelDmX, pixelDmY = self._sourProc.camXY2DmXY(xInpixelInCam, yInPixelInCam)

  # Get the sky position in (ra, decl)

  raInDeg, declInDeg = self._wcsSol.raDecFromPixelCoords(pixelDmX, pixelDmY, sensorName, )

  ( and the ts_wep bsc WcsSol.py did get updated 6 days ago https://github.com/lsst-ts/ts_wep/blob/develop/python/lsst/ts/wep/bsc/WcsSol.py ) 

• opdMetrology  in addFieldXYbyCamPos  https://github.com/lsst-ts/ts_phosim/blob/e25ac907d0385b109caf1ee4a803d9f248b54096/python/lsst/ts/phosim/OpdMetrology.py#L328-L329 calls: 

  # Do the coordinate transformation

  fieldXInDegree, fieldYInDegree = sourProc.camXYtoFieldXY(xInpixel, yInPixel)

   

So I think that basically given the `WcsSol.py` update , and that `ts_phosim` skySim  uses it to get x,y --> ra,dec ,  it is good to update the test to reflect that.  Another question : why are we using two different functions for such similar functionality? 

I think that is just because I forgot to remove the duplicated functions. We could remove the duplication after we tested the simulation without the problem.

I just tested the latest `master` branch of ts_wep, phosim_utils, and ts_ofc v1.3.7 without the problems:

[root@05f728cb542d ts_phosim]# python bin.src/opdCloseLoop.py

Calculated PSSN is [0.59475346 0.589638   0.59052874 0.59292836 0.58497574 0.58628199

 0.59527559 0.58779044 0.58661072].

GQ effective FWHM is 0.5433.

Calculated PSSN is [0.96592765 0.96628028 0.96787884 0.966245   0.96562767 0.96761831

 0.96616062 0.96611983 0.96731786].

GQ effective FWHM is 0.1212.

Let me check the ts_ofc v2.0.0 and ticket branch of ts_phosim now.

I have this error:

[root@05f728cb542d ts_phosim]# python bin.src/opdCloseLoop.py

Traceback (most recent call last):

  File "bin.src/opdCloseLoop.py", line 42, in <module>

    closeLoopTask.runOpd(

  File "/home/saluser/ts_phosim/python/lsst/ts/phosim/CloseLoopTask.py", line 386, in runOpd

    self.configOfcCalc(instName, filterType, rotCamInDeg)

TypeError: configOfcCalc() takes 2 positional arguments but 4 were given

and this:

[root@05f728cb542d ts_phosim]# python bin.src/imgCloseLoop.py --eimage

/home/saluser/ts_wep/python/lsst/ts/wep/bsc/WcsSol.py:45: FutureWarning: Gen2 Butler has been deprecated (LsstCamMapper). It will be removed sometime after v23.0 but no earlier than the end of 2021.

  self._camera = obs_lsst.lsstCamMapper.LsstCamMapper().camera

CameraMapper INFO: Loading Posix exposure registry from .

Use the default OPD field positions to be star positions.

The star magnitude is chosen to be 15.

CameraMapper INFO: Loading Posix exposure registry from .

Traceback (most recent call last):

  File "bin.src/imgCloseLoop.py", line 43, in <module>

    closeLoopTask.runImg(

  File "/home/saluser/ts_phosim/python/lsst/ts/phosim/CloseLoopTask.py", line 811, in runImg

    self.configOfcCalc(instName, filterType, rotCamInDeg)

TypeError: configOfcCalc() takes 2 positional arguments but 4 were given

Tiago Ribeiro, did you update the files in ts_phosim/bin.src?

This update looks good to me. I have some comments that will send in email. Please consider them before the merge. The main part is here:

[Summary]

1. Please help to update the script in bin.src/ to make sure the simulation can run (the minimum is to check the OPD simulation can work and converge). Maybe you could let Chris to help to do the simulation and check the converge.

1. I have some concern to pass the filter type to python/lsst/ts/phosim/CloseLoopTask.py from the scripts in bin.src/. Please help to check this.

Some additional information requested by TeWei:

Result of running opdCloseLoop.py for comcam

result of running opdCloseLoop.py for lsstfam

result of running imgCLoseLoop for comcam

result of running imgCloseLoop for lsstfam

All the results are available at NCSA in /project/tribeiro/