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

Add fit_aimpoint notebook for 2025-03 #30

Merged
merged 6 commits into from
Mar 20, 2025
Merged

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Mar 4, 2025

Description

This adds a new analysis notebook fit_aimpoint_drift-2025-03.ipynb which updates the aimpoint drift model by adding two jumps, one in 2023:046 (Safe Mode) and a second on 2024:001 (ad hoc).

Unrelated fixes

Modernized the ruff config and made some benign changes.
Also remove pyrightconfig.json, which is only causing problems.

Related PR's

Interface impacts

Testing

Unit tests

  • No unit tests

Functional tests

The notebook includes functional testing. See also: sot/chandra_aca#189.

@taldcroft taldcroft requested review from jeanconn and javierggt March 5, 2025 17:46
@jeanconn
Copy link
Contributor

jeanconn commented Mar 10, 2025

Question on the notebook - I'm a little confused about why we have a drift model (the drift model code / the class definition) in the notebook and one in chandra_aca - should they share the same model (code)? If the calibration version in the notebook needs to be separate, some comment on that would be helpful.

@jeanconn jeanconn requested a review from Copilot March 10, 2025 16:35

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@taldcroft
Copy link
Member Author

Question on the notebook - I'm a little confused about why we have a drift model (the drift model code / the class definition) in the notebook and one in chandra_aca - should they share the same model (code)? If the calibration version in the notebook needs to be separate, some comment on that would be helpful.

I added this to the AcaDriftModel docstring in the notebook:

    Implementation note:
    - The class in this notebook is designed to work as a sherpa fit model. A key part
      of that is (1) initialization that processes a bunch of auxilliary data and (2) a
      ``__call__`` method that sherpa calls with the model parameters.
    - The `chandra_models.drift` model is designed to be initialized once with the drift
      model and then evaluated with the ``calc`` method. Even though the basic model
      computation is equivalent, it is not easy to re-use this class for the sherpa
      fitting process.

@taldcroft taldcroft merged commit 0a48146 into master Mar 20, 2025
2 checks passed
@taldcroft taldcroft deleted the aimpoint-drift-2025-03 branch March 20, 2025 18:56
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