Skip to content

Remove text referencing scalar grid types#184

Open
mcflugen wants to merge 6 commits into
stablefrom
mcflugen/remove-scalar-grid-type
Open

Remove text referencing scalar grid types#184
mcflugen wants to merge 6 commits into
stablefrom
mcflugen/remove-scalar-grid-type

Conversation

@mcflugen

@mcflugen mcflugen commented Jun 2, 2025

Copy link
Copy Markdown
Member

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

Copilot AI left a comment

Copy link
Copy Markdown

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,

Copilot AI Jun 2, 2025

Copy link

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
Copy Markdown
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

mcflugen commented Jun 2, 2025

Copy link
Copy Markdown
Member Author

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