Skip to content

Stop fetching and parsing thermal constraint file#68

Merged
jeanconn merged 2 commits intomasterfrom
missing-load
Feb 17, 2022
Merged

Stop fetching and parsing thermal constraint file#68
jeanconn merged 2 commits intomasterfrom
missing-load

Conversation

@jeanconn
Copy link
Copy Markdown
Contributor

@jeanconn jeanconn commented Jan 14, 2022

Description

Stop fetching and parsing thermal constraint file.

The thermal constraint file is no longer created by FOT thermal as the outputs are no longer relevant / no longer based on models used by the FOT. The file used to be included in the FOT approved products outputs.

Testing

  • [n/a] Passes unit tests - unit tests have not been modernized and are presently disabled in Replan Central.
  • Functional Testing:
    In a test version (see wiki for howto) this PR removed the warning "Could not find constraints file for any load version of therm_JAN1022" from the top and the load link was appropriately populated with the link to the approved JAN1022A directory.

@jeanconn jeanconn changed the title WIP: Stop fetching and parsing thermal constraint file Stop fetching and parsing thermal constraint file Jan 18, 2022
@jeanconn
Copy link
Copy Markdown
Contributor Author

Given scope, I don't think we need to capture more test outputs etc in the PR.

@jeanconn
Copy link
Copy Markdown
Contributor Author

There are probably a few other places where text warnings could be updated to remove anything about "thermal violation events".

@taldcroft
Copy link
Copy Markdown
Member

Yeah, I see at least a couple of or thermal that should be removed. So it's worth searching the code for thermal and assessing if matches still make sense.

@taldcroft
Copy link
Copy Markdown
Member

Otherwise this looks relatively uncontroversial. It's been awhile, so (if it's not too complicated) could you remind me of how you actually ran this in a test version? (And put this in the PR description in a Functional testing section).

And as always it is useful to put in a little backstory about why you are doing this.

@jeanconn
Copy link
Copy Markdown
Contributor Author

Right, so for scope if the thermal constraint report isn't available are there any thermal violations that could be tracked by the code?

@jeanconn
Copy link
Copy Markdown
Contributor Author

With regard to testing, we've got setup notes on the wiki https://github.com/sot/arc/wiki/Set-up-test-Replan-Central . I figured that there might be other fixes in a replan central release that would go to FSDS and get more test output review.

@taldcroft
Copy link
Copy Markdown
Member

if the thermal constraint report isn't available are there any thermal violations that could be tracked by the code?

I don't believe there are any thermal violations in the PCAD constraint report.

@taldcroft
Copy link
Copy Markdown
Member

I figured that there might be other fixes in a replan central release that would go to FSDS and get more test output review.

Did you have some other changes in mind? Otherwise given the fairly slow release pace for arc this PR might be the driver.

Copy link
Copy Markdown

@malgosias malgosias left a comment

Choose a reason for hiding this comment

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

This PR looks good to me

@jeanconn
Copy link
Copy Markdown
Contributor Author

Thanks @malgosias !

@jeanconn jeanconn requested review from taldcroft and removed request for taldcroft January 21, 2022 21:31
@jeanconn jeanconn merged commit 491a145 into master Feb 17, 2022
@jeanconn jeanconn deleted the missing-load branch February 17, 2022 18:25
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