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 check to sanitize job timeseries data of NAN values #1778

Closed
wants to merge 2 commits into from

Conversation

aestoltm
Copy link
Contributor

Description

Add check to sanitize job timeseries data of NAN values due to Symfony throwing double NAN error when json encoding. Added helper function which replaces any NAN values in the series data with 0.

Note: This may need to be addressed in other areas. Need to find where else we are possibly letting NAN/null values through.

Motivation and Context

Charts with NAN data would return a null record.

Tests performed

Observed changes on xdmod-dev

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

…y throwing double NAN error when json encoding
@aaronweeden
Copy link
Contributor

What are example steps to reproduce the bug? I'd recommend including these in the "Motivation and Context" section above.

Copy link
Member

@jpwhite4 jpwhite4 left a comment

Choose a reason for hiding this comment

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

I really would prefer you change the JobMetadata->getJobTimeseriesData() implementation so that it does not return invalid values. - rather than having it still return nan data and then fudging it afterwards.

@aestoltm
Copy link
Contributor Author

Closed. Refer to PR: ubccr/xdmod-supremm#353

@aestoltm aestoltm closed this Sep 25, 2023
@aestoltm aestoltm deleted the fix_nan_values branch September 25, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants