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

Temporarily disable the inclusion of gratings moves states #43

Merged
merged 1 commit into from
Oct 25, 2021

Conversation

jzuhone
Copy link
Member

@jzuhone jzuhone commented Oct 25, 2021

Description

Recently, kadi was updated to include grating moves in the gratings states (sot/kadi#210). This changes not only the values of the grating states in the acis_thermal_check answer tests (expected) but also the values of the thermal propagation (unexpected, since the ACIS thermal models do not depend on grating state). The differences are very small (~0.1-0.2 degC) but are under investigation (a preliminary hypothesis suggests that the reason is that the JSON model spec files used for testing use evolve_method = 1).

For the time being, this PR disables the inclusion of grating moves states in acis_thermal_check, which ensures that the regression tests pass. This is ok from the perspective of operations assuming that the above hypothesis proves correct (since the flight versions of these files use evolve_method = 2) and since the actual temperature does not depend on gratings moves.

The tests and model specification files will be updated to accomodate the new gratings moves states at a date in the near future.

Testing

  • Passes unit tests on MacOS, linux, Windows (at least one required)
  • Functional testing

Copy link
Contributor

@Gregg140 Gregg140 left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Copy link
Contributor

@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.

LGTM

@jzuhone jzuhone merged commit 128bea9 into acisops:master Oct 25, 2021
@jzuhone jzuhone deleted the no_grat_moves branch February 23, 2022 22:09
@javierggt javierggt mentioned this pull request Aug 3, 2022
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.

4 participants