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

Low and mid-level functions #3

Open
mwtoews opened this issue Aug 8, 2023 · 12 comments
Open

Low and mid-level functions #3

mwtoews opened this issue Aug 8, 2023 · 12 comments

Comments

@mwtoews
Copy link
Collaborator

mwtoews commented Aug 8, 2023

Hi @jtwhite79 I have some parallel development going on:

  • ctypes_declarations.py - these are lowish-level ctypes stuff, no fancy classes
  • core.py - this is mid-ish-level logic for now that sends and receives basic Python and NumPy types with classes, but doesn't do too much higher-level pandas stuff. It's work in progress

It's parallel development, as you have similar interfaces in utils.py. We'll sort out how to harmonise the approaches at some point, but I don't want to step on your commits for now.

Here are some examples:

from pypestutils import core

lib = core.PestUtilsLib(logger_level=10)
# DEBUG:PestUtilsLib: loaded <CDLL '/data/mtoews/src/pypestutils/pypestutils-git/pypestutils/lib/libpestutils.so', handle 558649cfe590 at 0x7ff94c122370>
# DEBUG:PestUtilsLib: added prototypes

lib.install_structured_grid("foo", 3, 3, 4, 1, 1.1, 2.2, 0.0, 10, 20)
# INFO:PestUtilsLib: installed strictured grid 'foo' from specs

lib.uninstall_mf6_grid("foo")
# PestUtilsLibException: The name "foo" does not correspond to an installed MODFLOW 6 grid.

lib.install_mf6_grid_from_file("foo", "autotest/freyberg_structured/freyberg6.dis.grb")
# INFO:PestUtilsLib: installed mf6 grid 'foo' from grbfile='freyberg6.dis.grb'
# {'idis': 1, 'ncells': 2400, 'ndim1': 20, 'ndim2': 40, 'ndim3': 3}

lib.uninstall_mf6_grid("foo")
# INFO:PestUtilsLib: uninstalled mf6 grid 'foo'

it's going pretty smoothly, no crashes!

@jtwhite79
Copy link
Contributor

Nice Mike. I'll focus on a higher level class that uses the functionality...

@mwtoews
Copy link
Collaborator Author

mwtoews commented Aug 8, 2023

Sure, sounds good. I'll patch in the remaining Fortran functions and see what we can do with them.

@mwtoews
Copy link
Collaborator Author

mwtoews commented Aug 9, 2023

Ok, all 12 Fortran functions are now prototyped and exposed in pypestutils.core.PestUtilsLib. They will need fine-tuning and testing, but they are all working fine without crashing on Linux and Windows. Have a go with them and we'll see about any adjustments.

@mwtoews mwtoews closed this as completed Aug 9, 2023
@jtwhite79
Copy link
Contributor

@mwtoews were you planning to also add the model input functions as well? These listed in https://github.com/pypest/pypestutils/blob/develop/org_from_john_7aug2023/new_function_documentation.docx starting on page 28. Just curious...

@mwtoews
Copy link
Collaborator Author

mwtoews commented Aug 14, 2023

Yes, the plan is to go forward on those dozen other functions too. These will need some Fortran modifications to enable the bindings, then they can be prototyped and "Pythonized" along with the other functions. I'm only modifying in src (not orig_from*) so be careful not to run prep_src_dir.py. If/when John sends a new update, we'll need to prepare a cleaver patch to apply to keep the changes in src.

@jtwhite79
Copy link
Contributor

gotcha. I'm hoping we are done with prep_src_dir.py. Do you have ETA on those remaining functions?

@mwtoews
Copy link
Collaborator Author

mwtoews commented Aug 15, 2023

I've made a bit of progress, but it might be a few more days until they are all in. I'll push a few commits of the progress so far.

It's still mostly running smoothly, but there is potential trouble with (e.g.)

if(aa(i).lt.0.0) stop ' INVALID power variogram'

Since this is an external module, we'll probably need to check some of these inputs before passing. Otherwise the "stop" will kill the Python process, and I'm not sure if there is any elegant Fortran way to capture these signals.

@mwtoews mwtoews reopened this Aug 15, 2023
@mwtoews
Copy link
Collaborator Author

mwtoews commented Aug 15, 2023

I've added some enums too, which optionally allow their str counterparts, e.g.:

from pypestutils import core, enum
lib = core.PestUtilsLib(logger_level=10)

# These all do the same thing
lib.build_covar_matrix_2d([1.1, 2.2], [2.2, 5.4], 1, "spher", 1, 2, 3, 4, 5, 3)
lib.build_covar_matrix_2d([1.1, 2.2], [2.2, 5.4], 1, enum.VarioType.spher, 1, 2, 3, 4, 5, 3)
lib.build_covar_matrix_2d([1.1, 2.2], [2.2, 5.4], 1, 1, 1, 2, 3, 4, 5, 3)

As for the 1D array inputs, many of them can be array_like (e.g. a list), and others I'm relaxing to be scalars, e.g. zone 1 which gets filled-out to an array of 1 the expected length.

@mwtoews
Copy link
Collaborator Author

mwtoews commented Aug 15, 2023

And if you want to see gslib unexpectedly exit Python, here you go:

lib.calc_kriging_factors_3d([1.1], [2], [2], 2, [3.3], [3.4], [3.5], 2, 2, "simple", "pow", 2.2, 3.3, 4.4, 5.5, 6.6, 1.1, 2.2, 3.3, 10, 11, 3, "out.fac", "text")
STOP  INVALID power variogram

technically it's not a crash, but it is mishandled. It might be worth discussing with John, since he has already modified the gslib source for this project.

@jtwhite79
Copy link
Contributor

hmm. How many STOPs are we looking at? Im fine to refactor to call a stop function that returns nonzero and sets an error message...is that a workable solution?

@mwtoews
Copy link
Collaborator Author

mwtoews commented Aug 16, 2023

All of the remaining functions are in! Have a go, and gather your feedback.

Note that while "Pythonizing" the Fortran functions, the "in" (and "inout") parameters should match the Python signature, with a few exceptions:

  • Dimension sizes like npts is usually not needed, if it can be provided by another input array shape. Therefore, you won't need to supply some of these:
  • fieldgen2d_sva and fieldgen3d_sva have ldrand, which to me is not needed, since it should be the same as nnode. There are two other instances of leading dimension variables ldcovmat with build_covar_matrix_2d and build_covar_matrix_3d. Are these needed, e.g. is there a "known" size that it should have from the other inputs?

Furthermore, with the Python return value, if there is more than one "out" (or "inout"), then it is returned in a dict. But if there is only one output object, then that item is returned.

Other notes:

  • Mostly there are no defaults. Defaults can happen with a higher-level interface.
  • Some array values can be supplied as scalars (e.g. layer=1 or aniso=1.0) as a convenience, and some others expect an array (e.g. xcs=[1.0, 2.0]), since the array size is needed to define the array shapes. Lots of this magic is done in a private class, which also checks array dimensions, size, and ensures they are contiguous.

I'll write some pytests for these functions later, when we think we are happy with the current situation.

@jtwhite79
Copy link
Contributor

Nice one @mwtoews ! I'm pretty sure you can move ahead with pytests if you want - the higher level UI should be g2g with the way you've structured the underlying lib calls...

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

No branches or pull requests

2 participants