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

Issue #1158 remaining hfb bottlenecks #1159

Conversation

JoerivanEngelen
Copy link
Contributor

@JoerivanEngelen JoerivanEngelen commented Aug 16, 2024

Fixes #1158

Description

Fix remaining HFB bottlenecks. This reduces writing the HFB package from the LHM from 12.5 minutes to 2 minutes.

  • Cache results xu.Ugrid2d.from_structured, as this is a costly operation
  • Use and instead of & operator in the scalar_None function, to enable shortcutting.
  • Remove mask_all_packages function in from_imod5_data, call in tests.
  • Create separate pixi task for slow unittests, where just in time compilation is enabled. pixi run unittests still runs all unittests, by starting two pixi tasks unittests_njit & unittests_jit.

Checklist

  • Links to correct issue
  • Update changelog, if changes affect users
  • PR title starts with Issue #nr, e.g. Issue #737
  • Unit tests were added
  • If feature added: Added/extended example

@JoerivanEngelen JoerivanEngelen changed the base branch from master to imod5_converter_feature_branch August 16, 2024 15:59
Copy link
Contributor

@luitjansl luitjansl left a comment

Choose a reason for hiding this comment

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

looks good.1 comment, approving in advance

@@ -1378,8 +1378,5 @@ def from_imod5_data(
)
simulation["ims"] = solution

# cleanup packages for validation
idomain = groundwaterFlowModel.domain
simulation.mask_all_models(idomain)
Copy link
Contributor

Choose a reason for hiding this comment

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

there are a few examples on regridding. Even if they don't need this masking, it may be helpful to modify them to do it anyway to make the reader aware how to mask all the regridded packages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The hondsrug example is the example where we regrid a simulation. In this example data is already made consistent, and regridding doesn't introduce any problems. I defer from adding unnecessary calls to the example, as it might lead users to blindly call computationally intensive functions. However, I want to create an example how to import a (problematic) iMOD5 model, where some cleanup needs to be done (for which cleanup utilities have to be made). We could introduce that here. I'll create a separate issue for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See issue here #1164

Copy link

sonarcloud bot commented Aug 20, 2024

@JoerivanEngelen JoerivanEngelen merged commit ccf9f21 into imod5_converter_feature_branch Aug 20, 2024
6 checks passed
@JoerivanEngelen JoerivanEngelen deleted the issue_#1158_remaining_HFB_bottlenecks branch August 20, 2024 09:33
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.

Bottlenecks writing HFBs LHM
2 participants