NREL / SSRS

A stochastic agent-based model for predicting raptor movements during updraft-subsidized directional flights
https://www.osti.gov/doecode/biblio/63192
BSD 3-Clause "New" or "Revised" License
11 stars 5 forks source link

Some refactoring suggestions #11

Open rthedin opened 2 years ago

rthedin commented 2 years ago

I have some suggestions that I think it will improve code maintenance and debugging. None of this is exactly needed at this point, but if time becomes available, it would be nice to have.

  1. Exception handling. When the code checks if a file already exists in order to load it (instead of downloading or computing it again), it is done in a try/except way. The exceptions are also caught generically, on a catch-all umbrella (e.g. except Exception as _). https://github.com/NREL/SSRS/blob/1cda6625f665b8a22e7db454d36babfd4643364a/ssrs/simulator.py#L161-L165

    I think we should first check if the file exists, then read it if it does. Only then, we can handle exceptions if something goes wrong. We should not fall into an exception if the file does not exist, since that will happen the first time running it. These functions should return some False/True, instead of carrying over a generic exception. In some other instances, computations are performed within a try/except block and I think that should really be removed. When something goes bad, the actual exception is never given to the user.

  2. We should really consider moving grid data to an xarray format. The rasterio operations can probably be handled by rioxarray . Problems with orientation of aspect and wind direction conventions are handled ad-hoc. For example, this should be removed: https://github.com/NREL/SSRS/blob/1cda6625f665b8a22e7db454d36babfd4643364a/ssrs/layers.py#L560 https://github.com/NREL/SSRS/blob/1cda6625f665b8a22e7db454d36babfd4643364a/ssrs/layers.py#L579

  3. The simulator class has too many methods that should not be there. For example, all the plotting routines should be moved to another file/class.

  4. kwargs keywords should be added to plotting functions, which are then passed to the actual pcolormesh calls.