MassimoCimmino / pygfunction

An open-source toolbox for the evaluation of thermal response factors (g-functions) of geothermal borehole fields.
BSD 3-Clause "New" or "Revised" License
48 stars 21 forks source link

Create API for computing g-function values from static inputs #308

Open mitchute opened 1 week ago

mitchute commented 1 week ago

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 commented 1 week ago

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? :wink:

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

@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 commented 5 days ago

@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.

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...)