Fix for get_mix_forecast ValueError: cannot convert float NaN to integer#502
Conversation
|
Hi Paul, thanks for the fix. However I think this is not needed, you just need to add your sensor name to the |
|
Hi David, I just checked and I already have the PV sensor listed under sensor_replace_zero but it is still getting a NaN value at that point in the code. I only just started getting these errors with version 0.13 so thought it might be the PV adjustment code that might be generating the NaNs. I'll have another look. |
|
Hi David, I did some debugging/logging today and think I have found the source of the issue. The fix I proposed in this PR resolved the issue for me, but I think there is an underlying issue to be resolved. I saw that, as you mentioned, the sensor_replace_zero config option should have prevented this and I did have my PV sensor listed there. I traced the data and saw that the retrieve_hass.py prepare_data method looks like it is supposed to do this via the var_replace_zero list. I added some extra logging and saw it remove my PV sensor from the var_replace_zero list: It seems to be due to code added recently (which explains why it worked for me in 0.12.8). If I read it correctly, it appears to empty var_replace_zero and var_interp unless they contain all of the sensors in self.var_list. In my case, and I'm still an emhass newbie so it may be incorrect, I have different sensors in those 2 lists. When I edited config.json to change that section from this: ... to this (so the lists were the same): ... the debug logging shows the lists were retained: It then replaced the missing PV values with zeros, as expected, and continued on successfully. I'm wondering if that recent change in retrieve_hass.py prepare_data method is incorrect and it should only be removing individual sensors that are not in the var_list? If so then, as you said, this PR becomes unnecessary when that is fixed. |
|
Actually I wonder if it was my addition of the "sensor.test_p_pv_forecast" into my config.json (as part of using the new PV adjustment in 0.13) that triggered this exception (and highlighted this issue). The recent code changes to the retrieve_hass.py prepare_data method would have already been in in 0.12.8. |
|
Do you think that instead of: ... it should be (switching the order of the 2 lists): Or better still, only remove those sensors from var_replace_zero that are not present in self.var_list (and log a warning) rather than empty the entire list? The same goes for the code that processes var_interp too I think. |
|
Hi Paul thanks for looking deeply into this. In my case I need to deep dive to find the root cause of the issue. |
|
Hi David, I'll certainly try add a unit test. I had some knowledge issues previously in trying to get the unit tests running (I run everything in docker) but I'll have another try with this as it would be good to be able to run the unit tests. |
So do you think that this should do the trick? |
… list and missing data zero replacement handling. Added test_prepare_data_missing_pv unit test to verify. Reverted the now redundant fix from Forecast get_mix_forecast
|
Hi David, That replacement code worked nicely. Thanks. I've added a related unit test as suggested and reverted the original redundant change from earlier. Something you may notice in the test, that tripped me up initially, is that I had to copy the var_list from the test into the RetrieveHass instance. It looks that doesn't get populated when the data is loaded from the file data/test_df_final.pkl in setup. |
|
Now I see other tests are failing and, looking at the warnings, I wonder if it is the same issue with the var_list. I'll look into it some more. |
|
It looks like the RetrieveHass prepare_data method relies on the RetrieveHass instances self.var_list being populated and it is not always being populated when the tests data are setup (via self.rh.var_list). Now that the prepare_data method var_replace_zero and var_interp lists are being validated against var_list, and their contents pruned based on that sometimes empty var_list, it is causing test failures. I could ignore var_list validation if it is empty when prepare_data runs, but it seems better to ensure it is populated as required by the callers. I have run out of time today so will try and have another look tomorrow. By the way, I am not running the unit tests against my actual HA instance yet (until I learn enough about the tests, I want to avoid them potentially making/publishing changes). My thinking being that they will run with data files and mocks like I imagine they do when running in github based on PR commit triggers. Is this a valid way to be running the tests? |
|
Thanks for looking into this. |
|
|
Thanks David, that's good to hear. I initially ran the tests in a VM with no network access to see what failed, and opensnitch to see what connections were attempted. I noticed a connection to open-meteo which was expected and a connection to myhass.duckdns.org which I redirected to localhost so it would fail fast. I ran through all the tests looking for where it was complaining about missing columns in the empty var_list and found all the instances (I think) where data was loaded from a pickle file and added lines to copy the loaded var_list into the rh.var_list and now all the tests are passing here. However, for some reason it is failing on github for macos-latest. I don't yet know why it is failing for macos but not ubuntu or windows. There is also a SonarCloud failure related to code duplication in tests/test_forecast.py - not sure what to do about that as it looks necessary. I used docs/develop.md to setup my testing environment and have some feedback/suggestions: In Step 2 - Develop / Method 1 - Python Virtual Environment it says to use pip with requirements.txt but there is no requirements.txt in the main directory - only one in the scripts dir. It looks like the runtime dependencies are When I initially tried to run the unit tests I got
Is this the correct way to setup Python Virtual Environment for testing now - should that step (or whatever else is best) be added to the doc? I can submit a PR if that helps. |
Hi Paul, yes this is the correct way using the |
As you can see the test is now passing for MacOs. From time to time some tests will fail annoyingly like this. A manual relaunch of the failed job typically fix it. The solarcloud fail can be ignored sometimes, depending on the type of failed check. |
|
So with this this PR should be good to go right? |
|
I think it is good to go. All of the unit tests are passing and I am now running it in my main emhass docker instance using the day-ahead and mpc optimizations with pv adjust. If there are any remaining issues it is likely to be from any scenarios not covered by unit tests where data is loaded from pickle files and the prepare_data method is used. |


When running emhass 0.13 with set_use_adjusted_pv: true I was finding that NaN values for PV after sunset were causing the following ValueError:
The line numbers above are out of sync with respect to version 0.13 forecast.py as this was a custom docker image with other changes (those from #499) in order to get set_use_adjusted_pv working in my install.
This change replaces NaN with zero which I assume is appropriate in this instance?