Create API for computing g-function values from static inputs

[mitchute]

This PR creates some structures to simplify using pygfunction as a g-function calculator. The API proposed collects the necessary data into a class, and provides a few methods and the necessary structure to enable passing static borehole field parameters as inputs, and returning the g-function values.

[j-c-cook]

Moving the interface from GHEDesigner into pygfunction? Much of this is already encapsulated in the gt.gfunction.gFunction object. This proposed api.get_g_function method has a nearly identical API to gt.gfunction.gFunction.

A proper gt.Borefield object would have an API that allows append(gt.borehole.Borehole). Once a Borefield object is created, it should be easier to interface into this package (from GHEDesigner). Massimo is diligent enough about his API that dependency declaration of pygfunction~=2.2 should be sufficient enough to maintain a wrapper in GHEDesigner. If you guys want new features in pygfunction~=2.3 etc. then what's wrong with modifying a line on your end? Essentially, should Massimo cater to every design tool that comes along? 😉

TLDR; Create Borefield in gt.boreholes. Scratch this new API module and politely ask GHEDesigners to manage interfacing their coordinates with Borefield on their own.

My two cents. Massimo supersedes.

[ikijano]

I like the idea of a simplified API but I think it would be more beneficial for all to create your own wrapper package with pygfunction dependency than patching pygfunction and increase maintenance load this end as @j-c-cook said.

Simplified access to pygfunction could help some especially those who don't need or want GHEDesigner.

[mitchute]

The intent is to create the a very simple interface for passing in a set of descriptive static parameters, and returning g-function values. This is a minimal addition that will provide easier access with near zero maintenance.

[MassimoCimmino]

Thank you @mitchute for the PR and thank you @j-c-cook and @ikijano for the input.

I agree having a more streamlined way of generating g-functions is a positive addition to the project. If I can summarize the PR, it is proposed to add a bore field class that (i) checks and formats geometrical parameters of the bore field and (ii) provides a class method to calculate the g-function. In addition, the bore field class can be initialized by lists of geometrical parameters.

Do we need a separate api module for these features or would it also suit your usage to implement the changes in the boreholes module? We already have the boreholes.field_from_file function. A boreholes.field_from_lists (or similar) function could be a nice addition to the module.

I would see basic usage as:

field = gt.boreholes.field_from_lists(x, y, H, D, r_b, tilt=0., orientation=0.)
gfunc = field.evaluate_g_function(alpha, time, method='equivalent', boundary_condition='similarities', options={})

Some points for discussion:

1. Is there any specific reason to use lists instead of arrays? The rest of the package relies heavily on numpy. An option could be for the classes and functions to expect arrays but accept lists (like numpy usually does).
2. Is it necessary to separate the methods __init__ and initialize_borehole_field_generic?
3. I would argue for the evaluate_g_function method to have the same default parameters as in the gFunction class. We could add a parameter (e.g. output='object') for the user to decide whether to return the g-function as a g-function object, an array, or a list.

Let me know your thoughts. This could provide the basic interface before later working on the integration of the bore field class into the other modules (in #210).

[j-c-cook]

@MassimoCimmino Considering the order of modules listed in __init__.py, and gfunction.pys dependence on boreholes.py, what you have proposed may result in a circular import dependency issue. Please consider the following alternative, where gfunc is the evaluated g-function when output = 'result', and gFunction object when output = 'object'.

field = gt.boreholes.field_from_lists(x, y, H, D, r_b, tilt=0., orientation=0.)
gfunc = gt.gfunction.get_g_function(field, alpha, time, method='equivalent', boundary_condition='similarities', options={output='result'})

Here's my response to your points:

1. I agree it would be nice to allow either numpy array or list (like numpy and matplotlib do).
2. They may want to carry around a single gt.boreholes.Field object, and modify it throughout the design process. The current __init__ function allows that creation early on for them. initialize_borefield_generic is a function they could call multiple times as the coordinates and then heights are adjusted. A name change may be suitable, perhaps update.
3. I like your idea of returning either the object or the gfunction itself based on the kwarg variable output. Though, like mentioned above, I propose for that method to be located inside of the gfunction module.

[mitchute]

@MassimoCimmino thanks for supporting this effort. Your summary of the PR is accurate. Also, thank you for pointing out issue #210. I think this aligns well with that.

I'll try to respond to your points below.

• I'm OK with putting this wherever you prefer it be. In my opinion, having it be in api creates a clear signal to the user that this is what they should be using. A second possible option would be to put it in a new file called borefield or similar, since as I understand it, boreholes generally relates to single instances of boreholes, and this would encompass functionality related to the collection of boreholes. We could also put it in boreholes if you prefer, though that file is already getting a somewhat long. Also, I generally prefer to adhere to one-class-one-file just to keep things simple.

• The reason we are not using __init__ is to eventually allow multiple methods for construction. One idea would be to integrate all of the standalone functions in boreholes for defining the field arrangement (rectangular, L, U, etc.). This would unify how these are constructed and used.

field_1 = BoreField()
field_1.init_l_shaped(L_shaped_args...)
field_1.generate_g_functions(args...)

field_2 = BoreField()
field_2.init_u_shaped(U_shaped_args...)
field_2.generate_g_functions(args...)

• I'm OK with making the function take numpy arrays or lists. If they are using pygfunction they will already have numpy, but I would prefer we do not require use of numpy just to interact with this interface.

• I'm OK with making function arguments mirror the gFunction class. If you think there's a use case for needing to return the class instance itself vs. the actual list of g-function values, then I think those should be separated with different function calls, such as get_gFunction_instance and get_g_function_values. Having the return type be a conditional switch just seems like more complication than is needed.

[MassimoCimmino]

I quickly drafted an implementation on my branch : https://github.com/MassimoCimmino/pygfunction/tree/api

1. I agree with placing the Borefield class in its own module. To follow on @j-c-cook's comment, the circular dependency can be avoided if the gFunction class is imported when evaluate_g_function() is called rather that when importing the module.
2. You can use the @classmethod decorator to define multiple constructors for the same object. In this case, an example use is:

from gt.borefield import BoreField
# A list of boreholes is the default initialization argument
field_1 = BoreField(list_of_boreholes)
gfunc_1 = field_1.evaluate_g_function(field, alpha, time)

# Class methods build the list of boreholes internally to create the object
field_2 = BoreField.from_lists(H, D, r_b, x, y)
gfunc_2 = field_2.evaluate_g_function(field, alpha, time)

We can definitely add the field arrangements to the Borefieldclass. For the moment, the functions will need to be duplicated and included in the list of deprecations for v3.0.
3. numpy.typing provides the convenient npt.ArrayLike that covers floats, lists and arrays.
4. I only implemented evaluate_g_function, which returns the gFunction object. I could add a evaluate_g_function_values if it is deemed more convenient than:

gfunc = field.evaluate_g_function(field, alpha, time).gFunc

[mitchute]

@MassimoCimmino thank for this. This all sounds fine.

How do you want to proceed here? Should we pull your changes into our branch, or do you just want to go ahead with yours?

[MassimoCimmino]

@mitchute I thinkk I can push directly to your branch if that is fine with you. I will tidy up the implementation and come back to you for a final look before merging.

I may change it so that the default __init__ is actually from the lists and that .from_boreholes(args) is an alternative initialization. This way the interfaces to the Borehole and Borefield classes are similar. It should also be more efficient since with the current implementation, .from_lists(args) creates a list of boreholes, then calls __init__ which rebuilds the arrays from the list of boreholes. Using lists directly also prepares better for #210.

[mitchute]

@MassimoCimmino that works for me, thanks!

[MassimoCimmino]

Copying my comment from mitchute#4:

A little summary of my implementation if you want to discuss any of the changes:

1. The gFunction class now accepts Borefield objects. That did not require changes in the gFunction class (outside of isinstance checks) since the new class behaves like a list of boreholes for all operations done in pygfunction.
2. The Borefield.evaluate_g_function class method returns an array of g-function values. This was to align with the current gFunction.evaluate_g_function method.
3. I moved borefield creation functions (e.g. boreholes.rectangle_field) to the Borefield class. They needed to be re-implemented to generate arrays of geometrical parameters instead of lists of boreholes. The functions in the boreholes module now have a deprecation warning.
4. Examples are updated to use the new interface for bore field creation.

Something to consider is if the boreholes.visualize_borefield should also be moved to the Borefield class. I remember we had a discussion about the dependency for matplotlib. An option is to include the method and import matplotlib at runtime within the method instead of the top of the module.

The PR is much larger than I expected. I will review it and make sure it is compatible with the planned changes for issue #210.

[mitchute]

Something to consider is if the boreholes.visualize_borefield should also be moved to the Borefield class. I remember we had a discussion about the dependency for matplotlib. An option is to include the method and import matplotlib at runtime within the method instead of the top of the module.

Either way seems OK for now, but the latter option may be better. I believe we have some changes drafted proposing to move matplotlib to an extras install option. Once this merges, we can pull in the latest changes and submit that PR for your review.

[MassimoCimmino]

Either way seems OK for now, but the latter option may be better. I believe we have some changes drafted proposing to move matplotlib to an extras install option. Once this merges, we can pull in the latest changes and submit that PR for your review.

In that case I will place it at the top of the file and we can iterate in a separate thread.

[MassimoCimmino]

Merged. Thanks to all who contributed.