Repository navigation
Add check_resources hook for resource checks in check_case_data - #109
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The linked issue’s element-level hook and resource log grouping are missing, and two documentation links are unresolved.
3 open findings
What changed in this PR
Adds resource-level validation to the case consistency-checking framework.
Changes:
- Adds the
check_resourcesextension hook. - Adds resource-check tests and extension guidance.
- Updates internal documentation and release notes.
| File | Description |
|---|---|
src/checks.jl |
Implements and invokes the resource-check hook. |
test/test_checks.jl |
Tests custom resource validation. |
docs/src/how-to/extend-resource-functionality.md |
Documents implementing resource checks. |
docs/src/library/internals/functions.md |
Lists the new internal function. |
NEWS.md |
Records the new API. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
|
|
||
| # Check the resources of the case per resource family | ||
| for 𝒫ˢᵘᵇ ∈ res_types_vec(get_products(case)) | ||
| check_resources(case, 𝒫ˢᵘᵇ, modeltype, check_timeprofiles) |
There was a problem hiding this comment.
The question is whether that should be invoked directly here or could be a different PR. Given the description, it would not be necessary to have it here while I agree that Issue #107 states that it should be included. In this case, however, the question is where it is based located. As this is still up to debate, I would leave it at the moment be and ignore this comment.
| for 𝒫ˢᵘᵇ ∈ res_types_vec(get_products(case)) | ||
| check_resources(case, 𝒫ˢᵘᵇ, modeltype, check_timeprofiles) | ||
| end |
| - The individual elements vector must be unique, that it is not possible to have two vector | ||
| of nodes within the elements vector. | ||
| - Check that the coupling functions do return elements and not only an empty vector | ||
| - Call of [`check_resources`](@ref) for every resource family ([`resource_family`](@ref)) of |
JulStraus
left a comment
JulStraus
left a comment
There was a problem hiding this comment.
Not too many things to adjust. There is however an issue that we only consider ResourceEmits that are included in the case description. Any other Resources directly in the case description do not matter.
That is however something we have to consider separately.
|
|
||
| # Check the resources of the case per resource family | ||
| for 𝒫ˢᵘᵇ ∈ res_types_vec(get_products(case)) | ||
| check_resources(case, 𝒫ˢᵘᵇ, modeltype, check_timeprofiles) |
There was a problem hiding this comment.
The question is whether that should be invoked directly here or could be a different PR. Given the description, it would not be necessary to have it here while I agree that Issue #107 states that it should be included. In this case, however, the question is where it is based located. As this is still up to debate, I would leave it at the moment be and ignore this comment.
| for 𝒫ˢᵘᵇ ∈ res_types_vec(get_products(case)) | ||
| check_resources(case, 𝒫ˢᵘᵇ, modeltype, check_timeprofiles) | ||
| end |
…ase.jl into enhance/check_resources
Closes #107. Adds
check_resources(case, 𝒫ˢᵘᵇ, modeltype, check_timeprofiles), called fromcheck_case_datafor every resource family; the default checks nothing.check_case_datagains the argumentsmodeltypeandcheck_timeprofiles. See the issue for the discussion. Includes tests, NEWS entry and documentation.