c3-time-domain / SeeChange

A time-domain data reduction pipeline (e.g., for handling images->lightcurves) for surveys like DECam and LS4
BSD 3-Clause "New" or "Revised" License
0 stars 4 forks source link

Recursion error in image.merge_all #324

Closed rknop closed 1 month ago

rknop commented 3 months ago

Not sure what's going on, but here's how to see it:

On local machine, run a test environment (in "tests") with docker compose up -d shell and docker compose exec -it shell /bin/bash

Run the test:

   cd tests
   pytest -v improc/test_photometry.py -k test_background_sigma_clip

It passes.

Run it again: pytest -v improc/test_photometry.py -k test_background_sigma_clip

This time it fails:

_________________ ERROR at setup of test_background_sigma_clip _________________

>       lambda: ihook(item=item, **kwds), when=when, reraise=reraise
    )

/venv/lib/python3.11/site-packages/flaky/flaky_pytest_plugin.py:146: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
fixtures/ptf.py:172: in ptf_datastore
    ds = datastore_factory(
fixtures/pipeline_objects.py:953: in make_datastore
    ds.save_and_commit(session=session)
../pipeline/data_store.py:1560: in save_and_commit
    self.reference = self.reference.merge_all(session)
../models/reference.py:301: in merge_all
    new_ref.image = self.image.merge_all(session)
../models/image.py:617: in merge_all
    new_image.upstream_images[i] = im.merge_all(session)
../models/image.py:617: in merge_all
    new_image.upstream_images[i] = im.merge_all(session)
E   RecursionError: maximum recursion depth exceeded while calling a Python object
!!! Recursion detected (same locals & position)

It won't fail the first time in a fresh environment (which is why we don't see this failing on github actions), and it may pass every other time.

Worth investigation. Probably a pointer to an underlying problem.

rknop commented 3 months ago

...my reproduction may be overly simplistic. It worked for me a couple of times, but it may be stochastic rather than "every other time".

rknop commented 2 months ago

I haven't seen this recently, and whatever was causing it may have gone away.

rknop commented 1 month ago

No longer relevant, merge_all was removed with the database refactor.