spacetelescope / mirage

This code can be used to generate simulated NIRCam, NIRISS, or FGS data
https://mirage-data-simulator.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
39 stars 41 forks source link

Made max_frames hard coded value into a kwarg #775

Closed hover2pi closed 2 years ago

hover2pi commented 2 years ago

Moves hardcoded max_frames variable into SossSim.create keyword args so users can adjust memory allocation. Addresses #727 .

bhilbert4 commented 2 years ago

772 has been merged, so you should be able to bring those changes into this branch now. Those missing changes were causing the tests here to fail.

hover2pi commented 2 years ago

Hmmm, tests failing here:

FAILED tests/test_ra_dec_to_x_y.py::test_gal_rotations[0.0-galaxy_pos_angs0-expected_galaxy_pos_angs0]
FAILED tests/test_ra_dec_to_x_y.py::test_gal_rotations[30.0-galaxy_pos_angs1-expected_galaxy_pos_angs1]
FAILED tests/test_ra_dec_to_x_y.py::test_ext_rotations[30.0-ext_pos_angs1-expected_ext_pos_angs1]

Looks like an assertion error in the tests. Won't assert False always fail? Haha

        gal_x_pas = [c.calc_x_position_angle(pa) for pa in galaxy_pos_angs]
>       assert np.all(np.isclose(gal_x_pas, expected_galaxy_pos_angs))
E       assert False
E        +  where False = <function all at 0x7f08527d1940>(array([False, False, False]))
E        +    where <function all at 0x7f08527d1940> = np.all
E        +    and   array([False, False, False]) = <function isclose at 0x7f08527273a0>([59.317746739759755, 29.317746739759755, 139.31774673975974], [59.27713939017512, 29.277139390175122, 139.27713939017514])
E        +      where <function isclose at 0x7f08527273a0> = np.isclose

I haven't edited these tests or the code in this PR so I must have pulled it in when I merged. Do you know whats's happening here @bhilbert4 ? Thanks!

Same issue with #773.

bhilbert4 commented 2 years ago

Weird. Must be related to #767 although the tests were passing there when I merged it. I'll take a look.

bhilbert4 commented 2 years ago

@hover2pi I checked, and I think all you need to do is pull in the latest upstream/master and the tests here should pass.