Skip to content
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

Extend BMI interface #49

Closed
wants to merge 17 commits into from
Closed

Conversation

ahmad-el-sayed
Copy link
Collaborator

No description provided.

@Peter9192
Copy link

I tried building the docker container from this branch, but it appears to be broken due to recent changes in sfincs_infiltration. Do you think this is easy to fix, or could we rebase the changes from this PR onto https://github.com/Deltares/SFINCS/tree/a5136da72ad48e562dbaeba5ade2a9007f6c75c0 (I can confirm the docker build still worked there)?

@Peter9192
Copy link

Peter9192 commented Nov 5, 2023

Thanks a lot for all the work put into this! After struggling for some time I was able to use most of the new BMI functionality.

Main thing that didn't work as expected was the get_values implementation. I suggested a (partial) fix in #57. Another source of confusion was the grid type, it returned 'rectilinear', but that, in combination with grid_rank=2 suggests that grid_size = shape[0]*shape[1]. I changed it to always return unstructured in #57.

Finally a small note that update_until in the bmi spec gets the new target time, rather than the dt, as argument.

I've listed a few notes here

@Leynse Leynse closed this May 17, 2024
@Leynse Leynse deleted the feature/48-extend-bmi-functionality branch March 19, 2025 14:18
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