Improve indexing robustness and reduce memory use

COSIMA / cosima-cookbook

Framework for indexing and querying ocean-sea ice model output.

https://cosima-recipes.readthedocs.io/en/latest/

Apache License 2.0

57 stars 27 forks source link

Improve indexing robustness and reduce memory use #292

Closed aidanheerdegen closed 2 years ago

aidanheerdegen commented 2 years ago

Cherry picked some commits from #285 that are useful in and of themselves.

Namely:

Pass the open netCDF4.Dataset directly to update_timeinfo to avoid opening the file twice
Index only nfiles at a time (default 1000). This keeps memory use capped and makes indexing more fault tolerant

Memory is currently an issue. See https://github.com/COSIMA/master_index/commit/9c446ec33e0c42001ba588274cce4b356cc6c2ce

aidanheerdegen commented 2 years ago

Gawd, what a dog's breakfast of a PR, got there in the end.

aidanheerdegen commented 2 years ago

Bit of a dog's breakfast of a git history, but got there in the end! :)

Note that this still passes the sqalchemy 1.4 tests because it still has Scott's changes.

micaeljtoliveira commented 2 years ago

@aidanheerdegen I probably don't know the cookbook well enough to provide a meaningful review, but I wanted to tell you to not be afraid of interactively rebasing the branch to cleanup the git history. As long as you don't rewrite the history of master, all should be fine ;)

aidanheerdegen commented 2 years ago

Thanks @micaeljtoliveira. I usually do squash+merge but I can see the merits of cleaning the history with interactive rebasing, just not something I do a lot.

I asked for a review because it seemed polite, and the changes here are somewhat removed from the sqlalchemy inner workings, so thought another set of eyes is always helpful.

Also I haven't added any tests for the specific changes I've made. ~Probably~ Definitely should.