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

First version of the 1DPAMYT model and setup.py mods #72

Merged
merged 11 commits into from
Apr 29, 2024

Conversation

Gregg140
Copy link
Contributor

@Gregg140 Gregg140 commented Mar 12, 2024

Description

Initial version of a 1DPAMYT model which can be used if the 1DPAMZT sensor fails.

Interface impacts

The 1DPAMYT model will have a calling sequence similar to 1DPAMZT

Testing

Unit tests

  • No unit tests
  • Mac
  • [ x] Linux
  • Windows

Functional tests

Run with several past load weeks. Validation plots provided insight as to the goodness of the fit.

Copy link

@jazan12 jazan12 left a comment

Choose a reason for hiding this comment

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

Looked good to me

@jzuhone
Copy link
Member

jzuhone commented Mar 12, 2024

@Gregg140 can I push up a quick fix to this branch to fix the test failure? It's just a reformatting fix.

setup.py Outdated
author="John ZuHone",
author_email="john.zuhone@cfa.harvard.edu",
description="ACIS Thermal Model Library Program for 1DPAMYT",
author="Gregg Germain",
Copy link
Member

Choose a reason for hiding this comment

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

These lines in setup.py are for the whole package. They say me now, but we should probably put this instead:

author="ACIS Ops",
author_email="acisdude@cfa.harvard.edu",

Copy link
Member

Choose a reason for hiding this comment

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

and description="ACIS Thermal Model Library" should stay the same.

@Gregg140 Gregg140 self-assigned this Mar 13, 2024
@taldcroft
Copy link
Contributor

@Gregg140 - did you consider factoring out the common code into a base class to adhere to the DRY principle? The only functional difference between your new module and the existing one is changing the MSID name, so that MSID could be a base class attribute.

@Gregg140
Copy link
Contributor Author

@Gregg140 - did you consider factoring out the common code into a base class to adhere to the DRY principle? The only functional difference between your new module and the existing one is changing the MSID name, so that MSID could be a base class attribute.

John Z and I discussed this at length yesterday. John has identified a few places where we should consider a bigger refactor overall. And we agreed that we should put this present version in place for now so that we have it and can start running it sooner rather than later.

@jzuhone jzuhone merged commit 023f1ae into master Apr 29, 2024
1 check passed
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.

None yet

5 participants