Skip to content

Improve weather data handling - #198

Draft
kabarmaz wants to merge 11 commits into
mainfrom
improve-weather-data-handling
Draft

Improve weather data handling#198
kabarmaz wants to merge 11 commits into
mainfrom
improve-weather-data-handling

Conversation

@kabarmaz

Copy link
Copy Markdown
Contributor

Related Issue / Discussion:

Issue #13

Changes:

Please list the central functionalities that have been changed:

  • New Cache class, which utilizes hashable objects for a lookup table
  • Modified ship.py, Boat

Further Details:

Summary:

Please mention the relevant motivation and context of your changes and shortly describe the behaviour before and after your modifications.

Dependencies:

Please list new dependencies that are required for this modification.

PR Checklist:

In the context of this PR, I:

Please consider that PRs which do not meet the requirements specified in the checklist will not be evaluated. Also, PRs with no activities will be closed after a reasonable amount of time.

Comment thread WeatherRoutingTool/ship/ship.py Outdated
self._cache[key] = value
self._order.append(key)
# evict oldest if necessary
while len(self._order) > self.max_entries:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please drop the while, as every time the function is called only one entry needs to be deleted.

Comment thread WeatherRoutingTool/ship/ship.py Outdated
if max_entries <= 0:
raise ValueError("max_entries must be > 0")
self.max_entries = int(max_entries)
self._cache = dict()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please check whether the deletion of self._order and the replacement of self._cache by an OrderedDict increases the run-time performance. You can then use the command 'popitem(last=False)' to delete the first entry of _order.

Comment thread WeatherRoutingTool/ship/ship.py Outdated

def cached_lookup(var_key, da, lat, lon, t, height=None, depth=None):
key = (
var_key,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please convert the latitude, longitude and time values into indices and use these as keys to refer to the data in the lookup table.

Comment thread WeatherRoutingTool/ship/ship.py Outdated
self._lat_min = None
self._lon_min = None
self._lat_width = 1.0 / 12.0
self._lon_width = 1.0 / 12.0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

self._lat_min = None
self._lon_min = None
self._lat_width = 1.0 / 12.0
self._lon_width = 1.0 / 12.0

These variables are not used, are they? If not, they should be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants