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

Improve logging to screen and run.dat #65

Merged
merged 4 commits into from
Jul 25, 2023

Conversation

jzuhone
Copy link
Member

@jzuhone jzuhone commented Jul 17, 2023

Description

This PR improves the logging to screen and to the run.dat file that acis_thermal_check does at the beginning of its run. Specifically:

  • Removes the logging of the MD5 of the model specification to the screen since we don't really use it and this value can be different depending on how one serializes the model spec dict to JSON, even if the model is exactly the same. It adds more confusion than help.
  • Instead, if we don't supply a model specification file, log the chandra_models version we used to screen. If we do use this one, report chandra_models version as None and report the full path to the file.
  • Report the full path of the output directory that we used when printing out the command line options (currently it prints out the __repr__ of the directory which is a PosixPath).

For this sample command:

dpa_check --oflsdir=/data/acis/LoadReviews/2023/MAY2223/ofls --out=version_check

In the current version of acis_thermal_check, we see this:

acis_thermal_check: [INFO     ] # dpa_check run at Mon Jul 17 13:50:54 2023 by jzuhone
acis_thermal_check: [INFO     ] # acis_thermal_check version = 4.6.1.dev1+g32570dd
acis_thermal_check: [INFO     ] # model_spec file MD5sum = a363cf6438302298a1436e551719cabd
acis_thermal_check: [INFO     ] Command line options:
{'T_init': None,
 'backstop_file': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'days': 21.0,
 'interrupt': False,
 'model_spec': None,
 'nlet_file': '/data/acis/LoadReviews/NonLoadTrackedEvents.txt',
 'oflsdir': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'outdir': PosixPath('version_check'),
 'pred_only': False,
 'run_start': None,
 'state_builder': 'acis',
 'traceback': True,
 'verbose': 1,
 'version': False}

with this PR, we see this:

acis_thermal_check: [INFO     ] # dpa_check run at Mon Jul 17 13:49:37 2023 by jzuhone
acis_thermal_check: [INFO     ] # acis_thermal_check version = 4.6.1.dev3+g10bd3d6
acis_thermal_check: [INFO     ] # chandra_models version = 3.48
acis_thermal_check: [INFO     ] Command line options:
{'T_init': None,
 'backstop_file': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'days': 21.0,
 'interrupt': False,
 'model_spec': 'chandra_models v3.48',
 'nlet_file': '/data/acis/LoadReviews/NonLoadTrackedEvents.txt',
 'oflsdir': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'outdir': '/Users/jzuhone/version_check',
 'pred_only': False,
 'run_start': None,
 'state_builder': 'acis',
 'traceback': True,
 'verbose': 1,
 'version': False}

If we supply a model specification file like so:

dpa_check --oflsdir=/data/acis/LoadReviews/2023/MAY2223/ofls --out=version_check --model-spec=dpa_spec.json

In the current version of acis_thermal_check, we see this:

acis_thermal_check: [INFO     ] # dpa_check run at Mon Jul 17 13:52:34 2023 by jzuhone
acis_thermal_check: [INFO     ] # acis_thermal_check version = 4.6.1.dev1+g32570dd
acis_thermal_check: [INFO     ] # model_spec file MD5sum = 36a01389b6a909f81b4508f7c11a73fb
acis_thermal_check: [INFO     ] Command line options:
{'T_init': None,
 'backstop_file': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'days': 21.0,
 'interrupt': False,
 'model_spec': 'dpa_spec.json',
 'nlet_file': '/data/acis/LoadReviews/NonLoadTrackedEvents.txt',
 'oflsdir': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'outdir': PosixPath('version_check'),
 'pred_only': False,
 'run_start': None,
 'state_builder': 'acis',
 'traceback': True,
 'verbose': 1,
 'version': False}

with this PR, we see this:

acis_thermal_check: [INFO     ] # dpa_check run at Mon Jul 17 13:53:12 2023 by jzuhone
acis_thermal_check: [INFO     ] # acis_thermal_check version = 4.6.1.dev3+g10bd3d6
acis_thermal_check: [INFO     ] # chandra_models version = None
acis_thermal_check: [INFO     ] Command line options:
{'T_init': None,
 'backstop_file': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'days': 21.0,
 'interrupt': False,
 'model_spec': '/Users/jzuhone/dpa_spec.json',
 'nlet_file': '/data/acis/LoadReviews/NonLoadTrackedEvents.txt',
 'oflsdir': '/data/acis/LoadReviews/2023/MAY2223/ofls',
 'outdir': '/Users/jzuhone/version_check',
 'pred_only': False,
 'run_start': None,
 'state_builder': 'acis',
 'traceback': True,
 'verbose': 1,
 'version': False}

Interface impacts

Changes the visual appearance of the logging to screen and the run.dat file, but both of these are currently only human-read.

Testing

Unit tests

  • No unit tests
  • Mac
  • Linux
  • Windows

Functional tests

Checked to make sure that different combinations of specifying model-spec or not worked correctly.

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.

Looks alright to me.

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.

I approve as is.

I would consider not eliminating the "# chandra_models version = " line if a model spec is specified, but keeping it in there but saying something indicating why there's no model number. But that's a personal preference so ok if ignored.

@jzuhone
Copy link
Member Author

jzuhone commented Jul 25, 2023

I'm going to merge this and worry about the ruff checks in a separate PR, where hopefully I can get ruff to calm down.

@jzuhone jzuhone merged commit 090b176 into acisops:master Jul 25, 2023
@Gregg140
Copy link
Contributor

Still approved with the small mod to the Chandra Models Version line

@javierggt javierggt mentioned this pull request Sep 18, 2023
@javierggt javierggt mentioned this pull request Oct 4, 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.

3 participants