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

Improve output #297

Merged
merged 13 commits into from
Nov 30, 2023
Merged

Improve output #297

merged 13 commits into from
Nov 30, 2023

Conversation

rubencalje
Copy link
Collaborator

@rubencalje rubencalje commented Nov 24, 2023

This Pull Request does the following:

  • Add add_nans, hdry and hnoflo -arguments to nlmod.gwf.output.get_heads_da
  • Adds tests for groundwater transport, modpath, postprocessing of heads, geotop-methods and nlmod.plot.map_array
  • Solve bugs in notebooks and increase speed, so readthedocs will not fail anymore
  • Update url for watercourses of waterboard Hollands Noorderkwartier

@rubencalje rubencalje marked this pull request as ready for review November 29, 2023 09:28
@rubencalje rubencalje mentioned this pull request Nov 29, 2023
@@ -113,6 +113,28 @@ def xy_to_icell2d(xy, ds):
return icell2d


def xy_to_row_col(xy, ds):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will work on vertex grids as well, do we want to check for that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added check, also in xy_to_icell2d. But I think we should improve these methods, as they will give a wrong result in variable gridsizes, where the closest centre of the cell in not equal to the cell the point is in.

@@ -88,6 +88,12 @@ def _create_da(arr, modelgrid, times):
flopy modelgrid object
times : list or array
list or array containing times as floats (usually in days)
add_nans : bool, optional
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing major, but maybe dry_nans = True is a bit clearer, instead of add_nans.


df = nlmod.read.geotop.get_kh_kv_table()
gt = nlmod.read.geotop.add_kh_and_kv(gt, df)
# again, but using the stochastic method
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like stochastic is already tested in line before? Maybe modify comment to indicate why second test is different from first?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added more comments

Copy link
Collaborator

@dbrakenhoff dbrakenhoff left a comment

Choose a reason for hiding this comment

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

2 very minor comments.

Remove add_nans again, and just use the value for hdry or hnoflo to not add NaNs
@rubencalje rubencalje merged commit 098c680 into dev Nov 30, 2023
4 checks passed
@rubencalje rubencalje deleted the improve_output branch November 30, 2023 11:41
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