Bnb/misc #65
Bnb/misc #65
Conversation
…ined UWind, VWind features
…tion for fp outfiles
sup3r/pipeline/forward_pass.py
Outdated
| msg = (f'Full size of forward pass output ({fwp_out_mem / 1e9} GB) is ' | ||
| 'too large to hold in memory. Run with smaller ' | ||
| 'fwp_chunk_shape[2] or smaller spatial extent. ') | ||
| assert (mem.total - mem.used) > fwp_out_mem, msg |
There was a problem hiding this comment.
Recommend this as a warning instead of an assertion error. I'm pretty sure that sometimes python can hold memory and not free it, but it can use this memory for allocating new data. In any case, shouldnt we get an OOM error pretty quickly?
Also recommend you log the mem available in the msg for comparison purposes.
There was a problem hiding this comment.
Well currently the array to hold the fwp output is declared after the data extraction so it can take some time to get an OOM error. I could just move the declaration though.
There was a problem hiding this comment.
Actually I probably don't want to move the declaration bc that will restrict the mem available for the data extraction.
There was a problem hiding this comment.
Okay, well i dont love it but lets see how it goes haha
There was a problem hiding this comment.
This is really just to check if the data array that holds the fwp output is bigger than the total mem. If we are declaring an 900 GB array on a 800 GB machine itll fail no matter what magic python does
There was a problem hiding this comment.
*Bigger than the available memory. psutil is not perfect and I'd always rather be able to try stuff with a warning instead of the code not letting me.
There was a problem hiding this comment.
Yeah I could change it to total mem instead of available?
…um. changed output array mem assertition to warning.
| 'chunks': (2000, 500)}, | ||
| 'chunks': (2000, 500), | ||
| 'min': 0, | ||
| 'max': 100000}, |
There was a problem hiding this comment.
isnt this just 1 atm? If WRF is bar-absolute we should probably set an upper limit at like 1.5 * 1atm
There was a problem hiding this comment.
yeah missed a 1. good catch
There was a problem hiding this comment.
No what i'm saying is that WRF regularly produces pressures > 1atm so it shouldnt be 101325 Pa (1atm) it should be more like 1.5e5 Pa
There was a problem hiding this comment.
Yeah I meant I missed a one to make it 1.1e5 but I'll increase it further
There was a problem hiding this comment.
Oh gotcha, i saw 1.01e5 and thought you were setting the ceiling at exactly 1atm. sounds good!
Some small tweaks to accommodate new wtk data handling. Added physical limit enforcement on output handling.