Skip to content

feat: data initialisation, criteria and assessment parameters refactor #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Jun 4, 2025

Conversation

PeterBaker0
Copy link
Collaborator

@PeterBaker0 PeterBaker0 commented May 21, 2025

Missing criteria

  • adds wave height and period
  • makes all criteria params optional
  • uses the regional assessment criteria ranges explicitly to build out the query param dict
  • merges user provided ranges with the default from the dataset and warns if ranges are OOB

This has the effect of

a) allowing users to run jobs only restricting criteria of interest
b) ensuring regional and suitability assessment caches are shared where they should be, but invalidated if defaults change (by ensuring the query param dict passed to the hash function includes ALL criteria, not just those specified in the request)

Criteria, data initialisation and assessment params refactor

Rewrites the data initialisation, caching, criteria bounds etc all into a unified data structure which is fully typed. Did use structs all the way through but found that naming criteria in the structs ended up being over-engineered compared to having const global arrays/dicts of available criteria.

This has lots of side benefits like fixing regional assessment file caching, properly merging default ranges and user ranges (including into hash), and making criteria easy to add/exclude.

The process to add new criteria now should be easy i.e.

  • define the criteria metadata
  • specify which regions can use it

There is also a lot of additional runtime validation on data loading relating to presence of necessary fields.

Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
… basis

Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
… a dictionary mapping criteria ID to metadata and bounds - updating helpers etc

Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
@PeterBaker0 PeterBaker0 changed the title feat: improved criteria handling, merging with defaults, optional, missing wave height/period feat: data initialisation, criteria and assessment parameters refactor May 23, 2025
Copy link
Collaborator

@ConnectedSystems ConnectedSystems left a comment

Choose a reason for hiding this comment

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

Very minor comments, feel free to merge once addressed

PeterBaker0 and others added 10 commits May 28, 2025 16:55
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Co-authored-by: Takuya Iwanaga <takuyai@gmail.com>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
PeterBaker0 and others added 5 commits June 3, 2025 15:46
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
TODO: Double check the percentile values for Hs and Tp (waves)
Swap tooltip text

Co-authored-by: Peter Baker <87056634+PeterBaker0@users.noreply.github.com>
@PeterBaker0 PeterBaker0 merged commit deb6955 into main Jun 4, 2025
1 check failed
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.

2 participants