Skip to content

Conversation

@mcflugen
Copy link
Member

@mcflugen mcflugen commented Jun 2, 2025

What was described as a "scalar" grid type in the docs was really a variable without a grid. The correct way to deal with this case is to not assign a grid to these variables (i.e. their get_var_location should return "none"). None of the get_grid_ functions need to be implemented for these variables.

@mcflugen mcflugen requested a review from Copilot June 2, 2025 14:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the documentation for BMI grid functions by removing references to a "scalar" grid type, ensuring clarity on variables that do not utilize an actual grid.

  • Updated wording to reflect that exchange items may not be defined on a grid.
  • Removed the "scalar" grid type from the list of valid grid types.
Comments suppressed due to low confidence (1)

docs/source/bmi.grid_funcs.md:55

  • Ensure that the removal of the 'scalar' grid type is consistently reflected in any related documentation or usage examples for BMI grid functions.
- `scalar`

The functions in this section describe {ref}`model grids <model-grids>`.
In the BMI,
every {term}`exchange item` is defined on a grid,
if an {term}`exchange item` is defined on a grid,
Copy link

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

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

Consider clarifying that not all exchange items are defined on a grid, and explicitly mention that variables without a grid will have their get_var_location return 'none'.

Copilot uses AI. Check for mistakes.
@mcflugen mcflugen requested a review from mdpiper June 2, 2025 14:26
@PhilMiller
Copy link
Member

This is something that would only really be appropriate for a new major version of the standard, because it would make previously-conformant models that return scalar for some variables non-conformant.

@mcflugen
Copy link
Member Author

mcflugen commented Jun 2, 2025

This is something that would only really be appropriate for a new major version of the standard

Yeah, I should probably be merging this into the develop branch. Originally scalar variables were intended to work this way (and do for at least some BMI models) but the docs didn't reflect this.

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