search

sot / proseco

Probabilistic star evaluation and catalog optimization
https://sot.github.io/proseco
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

Pin chandra_models version to 3.48 for all tests #381

Closed jeanconn closed 1 year ago

jeanconn commented 1 year ago

Description

Pin chandra_models version to 3.48 for all tests with a pytest fixture that sets the environment variable.

Interface impacts

Testing

I configured with CHANDRA_MODELS_REPO_DIR to /Users/jean/git/chandra_models, set that chandra_models to be at pitch-roll-constraint-2023_020, set CHANDRA_MODELS_DEFAULT_VERSION to pitch-roll-constraint-2023_020, and added a throwaway tag in /Users/jean/git/chandra_models so it would still stop complaining that tip wasn't at tag 3.49. With that setup, I was reliably able to get the tests that (apparently) use the acquisition model to fail without this PR. I did not try to isolate acquisition model and pitch/roll changes.

Unit tests

Independent check of unit tests by @taldcroft (with the same setup noted above except without the "throwaway tag")

Functional tests

Unit tests are functional tests for this change.

taldcroft commented 1 year ago

I don't understand what's happening with the throwaway tag and problems that the tip was not at tag 3.49. I followed your setup exactly but did not do anything with making fake tags (and have never had to do that). The proseco tests passed in this case. I also did this to be sure I was getting what I thought:

In [9]: gfm = chandra_aca.star_probs.get_grid_func_model()

In [10]: gfm.keys()
Out[10]: dict_keys(['filename', 'func_no_1p5', 'func_1p5', 'mag_lo', 'mag_hi', 't_ccd_lo', 't_ccd_hi', 'halfw_lo', 'halfw_hi', 'info'])

In [11]: gfm["info"]
Out[11]: 
{'call_args': {'file_path': 'chandra_models/aca_acq_prob',
  'version': None,
  'repo_path': 'None',
  'require_latest_version': False,
  'timeout': 5,
  'read_func': '<function _read_grid_func_model at 0x118e82b90>',
  'read_func_kwargs': {'model_name': None}},
 'version': 'pitch-roll-constraint-2023_020',
 'commit': '9a43fe31c8c32415c648e7f82784d10009b0c935',
 'data_file_path': '/var/folders/zn/910d7qgd1ydd4bvww62b6b9r0000gp/T/tmphdte31ab/chandra_models/aca_acq_prob/grid-local-quadratic-2023-05.fits.gz',
 'repo_path': '/Users/aldcroft/git/chandra_models',
 'CHANDRA_MODELS_DEFAULT_VERSION': 'pitch-roll-constraint-2023_020',
 'CHANDRA_MODELS_REPO_DIR': '/Users/aldcroft/git/chandra_models',
 'md5': '2e8103341bb8def440d2b018548ca563'}

I reviewed the code again and from what I can see the only way to get to the tip exception is if version is None. This should not be happening if the CHANDRA_MODELS_DEFAULT_VERSION is set.

jeanconn commented 1 year ago

Did your setup have pitch-roll-constraint-2023_020 checked out in that chandra_models repo? You've said "followed your setup exactly", but what-the-repo-is-checked-out-at -- that seems to be my issue. I think the CHANDRA_MODELS_DEFAULT_VERSION works fine to get the right version of the model from the repo, but code complains if you aren't checked out at the last release. So I probably should have just checked out 3.49 instead of making a throwaway tag.

(ska3-matlab-2023.4rc6) spark:sparkles jean$ pytest -x --lf
========================================================================= test session starts ==========================================================================
platform darwin -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0
rootdir: /Users/jean/git/sparkles
plugins: timeout-2.1.0, anyio-3.6.2
collected 45 items / 1 error                                                                                                                                           
run-last-failure: None

================================================================================ ERRORS ================================================================================
____________________________________________________________ ERROR collecting sparkles/tests/test_review.py ____________________________________________________________
sparkles/tests/test_review.py:11: in <module>
    from proseco.characteristics import MonCoord, MonFunc, aca_t_ccd_penalty_limit
../../miniconda3/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/proseco/characteristics.py:97: in __getattr__
    _set_aca_limits()
../../miniconda3/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/proseco/characteristics.py:108: in _set_aca_limits
    spec, chandra_models_version = get_xija_model_spec(
../../miniconda3/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/xija/get_model_spec.py:107: in get_xija_model_spec
    model_spec, version = _get_xija_model_spec(
../../miniconda3/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/xija/get_model_spec.py:135: in _get_xija_model_spec
    version = get_repo_version(repo_path)
../../miniconda3/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/xija/get_model_spec.py:218: in get_repo_version
    raise ValueError(f"repo tip is not at tag {tag_repo}")
E   ValueError: repo tip is not at tag 3.49
=========================================================================== warnings summary ===========================================================================
../../miniconda3/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/chandra_aca/star_probs.py:349
  /Users/jean/miniconda3/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/chandra_aca/star_probs.py:349: UserWarning: 
  Model grid-* computed between mag <= 5.0 <= 10.75, clipping input mag(s) outside that range.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================================= short test summary info ========================================================================
ERROR sparkles/tests/test_review.py - ValueError: repo tip is not at tag 3.49
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
===================================================================== 1 warning, 1 error in 5.81s ======================================================================
(ska3-matlab-2023.4rc6) spark:sparkles jean$ echo $CHANDRA_MODELS_DEFAULT_VERSION
pitch-roll-constraint-2023_020

(git checkout 3.49 of chandra_models in another terminal)

(ska3-matlab-2023.4rc6) spark:sparkles jean$ pytest -x --lf
...
E       AssertionError: assert [{'text': 'Gu...': 'caution'}] == [{'category':...e requested'}]
E         At index 1 diff: {'text': 'P2: 3.48 less than 4.0 for ER', 'category': 'warning'} != {'text': 'P2: 3.33 less than 4.0 for ER', 'category': 'warning'}
E         Use -v to get more diff
...

After checking out at 3.49 I don't get the "tip" warning issue (though I suppose this doesn't confirm my default was behaving correctly at pitch-roll-constraint-2023_020 because this acq test is likely about the acquisition model update, but nevertheless).

taldcroft commented 1 year ago

Maybe the difference was that my version didn't have the 3.49 tag. I'll look into it.

taldcroft commented 1 year ago

Nope that wasn't it. What do you get with this? The only obvious diff now is that I'm using my ska3-dev env that has recent masters of relevant packages. In particular ska_helpers and chandra_aca should be recent.

export CHANDRA_MODELS_DEFAULT_VERSION=pitch-roll-constraint-2023_020             
export CHANDRA_MODELS_REPO_DIR=${HOME}/git/chandra_models
python -c "from chandra_aca import star_probs; print(star_probs.get_grid_func_model()['info'])"
taldcroft commented 1 year ago

Did your setup have pitch-roll-constraint-2023_020 checked out in that chandra_models repo?

Yes.

jeanconn commented 1 year ago 
 python -c "from chandra_aca import star_probs; print(star_probs.get_grid_func_model()['info'])"
{'call_args': {'file_path': 'chandra_models/aca_acq_prob', 'version': None, 'repo_path': 'None', 'require_latest_version': False, 'timeout': 5, 'read_func': '<function _read_grid_func_model at 0x117a437f0>', 'read_func_kwargs': {'model_name': None}}, 'version': 'pitch-roll-constraint-2023_020', 'commit': '9a43fe31c8c32415c648e7f82784d10009b0c935', 'data_file_path': '/var/folders/63/ljv0yn5x01x6xf4cymvh814m0000gn/T/tmpqyw27mog/chandra_models/aca_acq_prob/grid-local-quadratic-2023-05.fits.gz', 'repo_path': '/Users/jean/git/chandra_models', 'CHANDRA_MODELS_DEFAULT_VERSION': 'pitch-roll-constraint-2023_020', 'CHANDRA_MODELS_REPO_DIR': '/Users/jean/git/chandra_models', 'md5': '2e8103341bb8def440d2b018548ca563'}
jeanconn commented 1 year ago

I'm not seeing the issue right now in proseco; my pasted output above was from where I moved on to sparkles. I think the conftest.py test fix is still the right thing either way.