MLOS icon indicating copy to clipboard operation
MLOS copied to clipboard

Enforce float, avoid crashing optimizer. If value is NAN will cause optimizer to crash

Open yshady opened this issue 1 year ago • 2 comments
trafficstars

yshady avatar Jul 05 '24 19:07 yshady

@microsoft-github-policy-service agree

yshady avatar Jul 08 '24 21:07 yshady

a few things:

  • There are pylint and pycodestyle violations. Run all these checks locally to make sure the code conforms to our coding standards.
  • There are many changes that are unrelated to the fix (indentation, parameter updates, etc.). We usually split them out into separate PRs.
  • Most importantly, I doubt that we need that fix at all. We somehow got some garbage in the database, and it is completely legitimate that we complain about it now. I think the right solution would be to clean up the DB and remove the offending records (and investigate how they've got there) instead of quietly coercing data to NaNs.

@bpkroth, what do you think?

motus avatar Jul 08 '24 22:07 motus

@motus the float fixes @yshady are proposing are due to this issue described here in more detail: #785

The fix proposed here (some kind of string to float conversion) should probably be applied earlier, before self._adjust_signs_df is called. In this case, we will just get a whole series of NaNs when bulk register is called.

eujing avatar Jul 10 '24 19:07 eujing

Thanks for identifying the issue! @eujing has filed an issue #785 describing the problem. Proper fix submitted in PR #789

motus avatar Jul 10 '24 21:07 motus