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

Imod6 1082 modflow6 packages optional #201

Closed

Conversation

rleander
Copy link
Contributor

Please review:

  • packages in modflow now optional: that means if coupling not active, no need to have the package in the mf6 model
  • pointers to kernel arrays now stored and retrieved from dictionaries
  • active couplings listed in log

@sonarcloud
Copy link

sonarcloud bot commented Oct 31, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 18 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@rleander rleander changed the base branch from lumbricus to lumbricus_test2D_dfm2msw November 2, 2023 08:18
Copy link
Contributor

@HendrikKok HendrikKok left a comment

Choose a reason for hiding this comment

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

Most of the changes seem ok. The weird thing is that all functions from line 479 of dfm_metamod.py seem to be changed (at least they are red) but they are not as far as I can see. Can you changes this maybe? For now its hard for me to review that part of the changes at this point. Perhaps a rebase first?

@@ -155,7 +172,8 @@ def set_mapping(self) -> None:

def update(self) -> None:
# heads from modflow to MetaSWAP
self.exchange_mod2msw()
if self.map_mod_msw["mod2msw_head"] is not None:
self.exchange_mod2msw()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be optional? This affects the core coupling between MF6 and MEtaSWAP?

"mf6_riv_active": self.mf6.get_river_drain_flux_estimate(
).size
try:
array_dims["mf6_riv_active"] = self.mf6.get_river_drain_flux_estimate(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not better to check if the specific package is defined in TOML-file? There is no need to test the function itself right? The function only doesn't work if the package names are None?

"msw-ponding2dflow2d_flux",
self.dfm.get_current_time_days(),
)
self.matrix_product(
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the check is now only done in matrix_product() ?


# for calculating the realised ponding volume, the flux need to be split up in positive and negative values
Copy link
Contributor

Choose a reason for hiding this comment

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

what changed here? All changes below don't seem to be changes right? Just a shift down? Can you change this maybe? Its hard for me to review all changes below...

super().__init__(lib_path, lib_dependency, working_directory, timing)

def get_value_ptr(self, name: str) -> NDArray:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this in the wrapper? This is standard xmi right?

@rleander rleander closed this Jul 3, 2024
@rleander rleander deleted the imod6-1082-MODFLOW6_packages_optional branch July 3, 2024 07:45
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.

2 participants