Skip to content

Add caching options#412

Merged
dbrakenhoff merged 11 commits intodevfrom
add_caching_options
Feb 28, 2025
Merged

Add caching options#412
dbrakenhoff merged 11 commits intodevfrom
add_caching_options

Conversation

@dbrakenhoff
Copy link
Collaborator

This allows overriding caching behavior using nlmod.config.NLMOD_CACHE_OPTIONS. This allows you to turn off the hash checks, and optionally activate an explicit dataset coordinate comparison. This allows some more flexibility when running into issue #389.

Recommended usage:

import nlmod

# ... define some ds

# temporarily turn off dataset coordinate hash check and turn on explicit check
with nlmod.config.cache_options(
    dataset_coords_hash=False,
    explicit_dataset_coordinate_comparison=True,
):
    nlmod.read.ahn.get_ahn(ds, cachename="ahn", cachedir=".")

add config submodule

  • define NLMOD_CACHE_OPTIONS
  • set defaults (identical to current caching defaults)
  • add context manager for cache options
  • add functions to set, get and reset options.

in cache.py

  • add check for global setting in NLMOD_CACHING_OPTIONS
  • add logging when global config modifies default
  • do not check hashes if global option overrides cache decorator setting
  • add explicit dataset coordinate check, useful if hash checks are disabled.

- define NLMOD_CACHE_OPTIONS
- set defaults (identical to current caching defaults)
- add context manager for cache options
- add functions to set, get and reset options.
- add check for setting NLMOD_CACHING_OPTIONS
- add logging when global config modifies default
- do not check hashes if global option overrides cache decorator setting
- add explicit dataset coordinate check, useful if hash checks are disabled.
@dbrakenhoff dbrakenhoff self-assigned this Feb 26, 2025
@dbrakenhoff dbrakenhoff linked an issue Feb 26, 2025 that may be closed by this pull request
@dbrakenhoff dbrakenhoff added the caching All caching related issues label Feb 26, 2025
Copy link
Collaborator

@OnnoEbbens OnnoEbbens left a comment

Choose a reason for hiding this comment

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

Just one comment and then you are good to go, should be an easy fix

@dbrakenhoff
Copy link
Collaborator Author

Good point about only invalidating the cache. This is the output now when you turn on the explicit coordinate check:
image

While writing this I realize that the caching should be as quiet as possible, so these ERROR messages should be downgraded to INFO I think. I think it is good to know why the cache is being invalidated once you choose this explicit route? But I could also live with DEBUG level. Lemme know what you think.

@dbrakenhoff
Copy link
Collaborator Author

I went for full debug level logging in the excplicit check.

Also, I removed "area" from the data set variables in ds_contains(). I think this was put there for resampling AHN data, but I think this variable can be easily computed if it does not yet exist.

I think this is good to go now.

@dbrakenhoff dbrakenhoff merged commit e042e71 into dev Feb 28, 2025
3 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in NHFLO Feb 28, 2025
@dbrakenhoff dbrakenhoff deleted the add_caching_options branch February 28, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

caching All caching related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache is never used because hashes do not match

3 participants