Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pin chandra_models version to 3.48 for all tests #381

Merged
merged 2 commits into from
Jul 7, 2023
Merged

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Jul 7, 2023

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

  • Mac

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

  • Mac

Functional tests

Unit tests are functional tests for this change.

@jeanconn jeanconn requested review from javierggt and taldcroft July 7, 2023 00:52
@taldcroft
Copy link
Member

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.

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jeanconn
Copy link
Contributor Author

jeanconn commented Jul 7, 2023

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
Copy link
Member

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

@taldcroft
Copy link
Member

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
Copy link
Member

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

Yes.

@jeanconn
Copy link
Contributor Author

jeanconn commented Jul 7, 2023

 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
Copy link
Contributor Author

jeanconn commented Jul 7, 2023

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.

@jeanconn jeanconn merged commit 4e2d19c into master Jul 7, 2023
@jeanconn jeanconn deleted the models-for-tests branch July 7, 2023 13:55
This was referenced Jul 12, 2023
@javierggt javierggt mentioned this pull request Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants