Skip to content

Atmosphere.updateLayer() ignores changes of telescope.samplingTime during a run (wind translation stays frozen) #74

Description

@2184746995rl-cpu

Atmosphere.updateLayer() freezes the wind-induced phase-screen translation when telescope.samplingTime is changed during a run

Environment

  • OOPAO version / commit: <git log -1 --format='%h %ad' 的输出>
  • Python: <3.x> , NumPy: <1.xx>
  • OS: <Windows/Linux/macOS>

Summary

Atmosphere.updateLayer() computes the per-frame wind translation

layer.ratio = [vX * telescope.samplingTime / pixel_size,
               vY * telescope.samplingTime / pixel_size]

only inside the if layer.notDoneOnce: branch, i.e. only on the first
update() call after a phase screen has been generated. From then on every
call reuses the stored layer.ratio, so if telescope.samplingTime is changed
later (as it can be by user code / an RL environment that treats the loop rate
as a control input), the turbulence keeps translating with the stale time
step and the loop rate has no effect on the temporal evolution of the
atmosphere any more.

Note that ps_turb_x / ps_turb_y are computed on every call, but the values
are only used the first time, which looks unintended.

Minimal reproduction

reproduce_atm_ratio.py (attached) builds a Telescope / Source / Atmosphere,
then calls update() four times while changing telescope.samplingTime
between the calls, and prints the translation that updateLayer() actually
applies:

call  sampling_rate   dt[ms]  expected[m/frame] expected[px/frame]  layer.ratio[1]
   1        1200Hz    0.833            0.01250              0.150          0.1500
   2         200Hz    5.000            0.07500              0.900          0.1500   <-- expected 0.900
   3         100Hz   10.000            0.15000              1.800          0.1500   <-- expected 1.800
   4        1200Hz    0.833            0.01250              0.150          0.1500

The last column stays at the value computed during call 1, although the time
step changes by a factor of 12. "expected" is simply
vY * telescope.samplingTime / layer.pixel_size with vY = 15 m/s
(layer_1 of the atmospheric profile used in the script).

Expected behaviour

layer.ratio should be recomputed from the current telescope.samplingTime at
every update, since samplingTime is a public attribute of Telescope
(documented as "AO loop speed") and nothing forbids changing it between frames.

Why this is easy to miss

In the standard closed-loop scripts (OOPAO/closed_loop/*) samplingTime is
set once in the Telescope constructor and never modified, so the cached value
is always correct there. The problem only shows up when the loop rate is
modified during a run.

Impact (why we care)

We use OOPAO as the physics backend of a reinforcement-learning AO environment
in which the WFS sampling frequency (hence telescope.samplingTime) is one of
the actions. With the current code, the first action of an episode decides
the temporal evolution of the whole episode: an agent that picks a high
sampling rate on the first frame gets a turbulence that appears ~5x slower per
frame for the remaining 500 frames, and this dominates the achievable Strehl
ratio (we measured SR 0.087 vs 0.475 with identical parameters, purely from
this effect). It also makes the environment non-Markovian in a way that is very
hard to diagnose: the reward at step t depends on the action taken at
step 0.

Suggested fix

Move the two ratio assignments out of the notDoneOnce branch (the branch is
still needed to initialise the sub-pixel accumulator layer.buff):

         else:
             if layer.notDoneOnce:
                 layer.notDoneOnce = False
                 layer.ratio = xp.zeros(2)
-                layer.ratio[0] = ps_turb_x/layer.pixel_size
-                layer.ratio[1] = ps_turb_y/layer.pixel_size
                 layer.buff = xp.zeros(2)

             if shift is None:
+                layer.ratio[0] = ps_turb_x/layer.pixel_size
+                layer.ratio[1] = ps_turb_y/layer.pixel_size
                 ratio = layer.ratio
             else:
                 ratio = shift    # shift in pixels

With this change the reproduction prints 0.150 / 0.900 / 1.800 / 0.150, and
closed-loop runs with a constant samplingTime are bit-for-bit unaffected.

Happy to open a pull request if you prefer.

Side note (unrelated, minor)

OOPAO/__init__.py locates (and writes) precision_oopao.npy with

OOPAO_path = [s for s in sys.path if "OOPAO" in s]
l = []
for i in OOPAO_path:
    l.append(len(i))
path = OOPAO_path[np.argmin(l)]
np.save(path+'/precision_oopao', 64)

which raises ValueError: attempt to get argmin of an empty sequence when the
package is imported from a directory whose path does not contain the string
"OOPAO" (e.g. a project folder named RL4AO/ao_sim, with the package on
sys.path as sys.path.insert(0, ".../ao_sim")). It also writes the precision
file into the first matching path, which may not be where the package actually
is. Using pathlib.Path(__file__).parent would be more robust. Not related to
the issue above.
``

reproduce_atm_ratio.py

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions