Skip to content

Conversation

@uramirez8707
Copy link
Collaborator

This PR:

  • Add a test that uses the refine ratio with a cubesphere to get full testing coverage
  • add hooks to be able to use logging instead of print statements
  • move the latlon specific stuff from make_grid_info into its own function
  • document the members of the hgridobj

… be able to use logging, move the latlon specific stuff from make_grid_info into its own function, documents the members of the hgridobj
@uramirez8707 uramirez8707 marked this pull request as ready for review September 18, 2025 13:35
self.jsc = None
self.jec = None

def __init_numpy_arrays__(self, nsuper, narea, ndx, ndy):
Copy link
Owner

Choose a reason for hiding this comment

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

do the arrays need to be initialized beforehand in python?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok init is a bad name, __allocate_numpy_arrays__?

This is allocating memory for all of the 1D members of HGridObj

They are set in pyfrenctools.make_hgrid_wrappers.create_*, which is why they are allocated here.

Copy link
Owner

Choose a reason for hiding this comment

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

I think the allocation can be abstracted into pyfrenctools.make_hgrid_wrappers.create_* (do the allocation there and return the appropriate arrays)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The way I see this turning out is

  1. For lat/lon grids in lonlat_grid.py
    -> grid_obj = HGridObj() (initiliazed the hgrid object)
    -> grid_obj.make_grid_info_lat_lon(...) (set up the arrays to hold the data)
    -> pyfrenctools.make_hgrid_wrappers.create_regular_lonlat_grid( ...) (fill in the arrays)
    -> grid_obj.write_out_hgrid(...) (write out the data)

  2. Simlarly for cubesphere grids in gnomonic_grid.py
    -> grid_obj = HGridObj() (initiliazed the hgrid object)
    -> grid_obj.make_grid_info_gnomonic(...) (set up the arrays to hold the data)
    -> pyfrenctools.make_hgrid_wrappers.create_regular_lonlat_grid( ...) (fill in the arrays)
    -> grid_obj.write_out_hgrid(...) (write out the data)

make_grid_info_lat_lon and make_grid_info_gnomonic are going to share code like __allocate_numpy_arrays__
make_grid_info_gnomonic has to do extra stuff (i.e grid_obj.x has a size of nlon * nlon * 6 for non nesting grids and nlon * nlon * 6 + nest_x * nest_y) for grids with nests

Let's talk about this tomorrow :)


self.logger.debug(f"make_grid_info: Creating a lat-lon with nlon={nlon} and nlat={nlat}")

self.nxl = np.array(nlon, dtype=np.int32)
Copy link
Owner

Choose a reason for hiding this comment

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

is nxl, nyl just scalars? They probably can just be self.nxl = nlon...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are 1D integers of size equal to the number of tiles. In this lat/lon case they are just one. The code that writes the output is currently expecting them to be arrays.

nx = self.nxl[n]
ny = self.nyl[n]

Copy link
Owner

Choose a reason for hiding this comment

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

ooo.... that should be changed...
(yup, unnecessarily complicated)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, there is a lot of unnecessarily complicated code.
I am just tackling this one step at a time.

ntiles = 6
ratio = 2
grid_size = 96
nlon = np.array([grid_size], dtype=np.int32)
Copy link
Owner

Choose a reason for hiding this comment

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

no need for scalars to be "np.array"'s

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