full size xg and yg

[AaronDavidSchneider]

This PR picks up on PR #195 and is currently WIP.
I plan to keep this PR minimal and not pick up the ability to reshape a dataset in tiles of a specified shape.
This PR only extends xg and yg to their full size.

[AaronDavidSchneider]

I will delay this PR until PR #98 is merged.
It makes no sense to do the work twice 😀

[AaronDavidSchneider]

Hi @rabernat,

I think I found a solution. In the current version of this PR we are extending extending i_g and j_g to nx + 1.
I think my implementation is straight forward and quite easy.

I tested it with a cs grid. But it should also work with llc.

Cheers
Aaron

[raphaeldussin]

I think this PR solves the issues of my previous implementation of this utility function

[AaronDavidSchneider]

Thanks @raphaeldussin for your feedback! I changed it to match your feedback.

[AaronDavidSchneider]

Hi @rabernat, @raphaeldussin, just a quick reminder that my work on this PR is finished.

[AaronDavidSchneider]

Hi, just another casual reminder @rabernat and @raphaeldussin (sorry for bothering).

[rabernat]

Thanks for the reminder @AaronDavidSchneider, and sorry for taking so long to review! We really appreciate your contribution. I will do my best to have a look this week.

[rabernat]

This is really great!

My one central concern is that we don't have a test. @raphaeldussin - do any of the test cases contain the needed input files to test this code?

[AaronDavidSchneider]

Thanks for your comments @rabernat!

[AaronDavidSchneider]

Regarding the tests:
It should be feasible to check if the XC and YC values are in the middle of the XG and YG values of outer=True.

What do you think?

[rabernat]

So I looked it up, and it seems that we already have a good test for the get_grid_from_input

xmitgcm/xmitgcm/test/test_utils.py

Line 961 in 4a9c748

def test_get_grid_from_input(all_grid_datadirs, usedask):

To this test you should add a section that calls the function with outer=True.

It should be feasible to check if the XC and YC values are in the middle of the XG and YG values of outer=True.

No, I would not recommend that. The data are coming from files hosted in the test data repo, so I would not make assumptions about the values. You just want to check that the arrays are the right shape.

Do you know how to run the test suite? Just run pytest from the root directory.

[raphaeldussin]

AaronDavidSchneider#1 should solve the problem.

[AaronDavidSchneider]

Thanks @raphaeldussin for your feedback and work on this PR.
It should work now.

[raphaeldussin]

getting very close to the finish line @AaronDavidSchneider

[AaronDavidSchneider]

Thanks for your feedback @raphaeldussin. I tried to address the proposed changes. I still have one open question (see above).

[AaronDavidSchneider]

So I just had a look into the llc checks in test_get_grid_from_input and tried to come up with a simple test that would do the same for the cs geometry.
However, the cs geometry is slightly different and I would need more knowledge about the raw data. E.g.

xc1 = read_raw_data(grid1, dtype=np.dtype('>d'), shape=(ny1, nx),
                            partial_read=True)

does not work.

I would suggest, that we merge this PR first (once its ready) and work on cs grid checks afterwards, since these checks are not really related to this PR anyway.

I would be happy to open up a new PR to work on these checks. However, I would need your help @raphaeldussin for the read_raw_data part.

[raphaeldussin]

I think this test on the values is important so we preserve the answers in any future code changes.
inferring/reading the raw data can do this way:

du -b file.bin and divide by 8 (number of bytes in double precision, which is the format used for grids), then nx+1 = 33 twice
since you have square faces of cs32 and you get 18 which is your number of variables. Then

read all variables:

raw = read_raw_data('grid_cs32.face001.bin', np.dtype('>d'), (18,33,33))

read only first one:

raw = read_raw_data('grid_cs32.face001.bin', np.dtype('>d'), (33,33), partial_read=True)

read second:

raw = read_raw_data('grid_cs32.face001.bin', np.dtype('>d'), (33,33), partial_read=True, offset=(33*33*8))

from there you should be able to add a section to the test for CS geometry.

[AaronDavidSchneider]

Thanks for the explanation @raphaeldussin! I took the llc check and adapted it for the CS grid. What do you think?

[raphaeldussin]

LGTM! @rabernat are you good with this as well?

[AaronDavidSchneider]

Hi @rabernat, could you have a final look on this PR?
Thanks!

[AaronDavidSchneider]

Hi @rabernat, could you have a final look on this PR?
Thanks!

[AaronDavidSchneider]

Just another kind reminder @rabernat.

[AaronDavidSchneider]

Hi @rabernat,
is there any possibility that this could be merged soon?

[rabernat]

Sorry for being extremely slow here. Just one question, then LGTM.

[AaronDavidSchneider]

Added precision and endianness to grid common.
Thanks a lot @rabernat and @raphaeldussin!

[rabernat]

Fantastic!

Reading the code, I realized I don't like how we have separate parameters for dtype and endian. We can handle endianness as part of dtype, so this is unnecessary. However, that predates your PR, so it doesn't have to be fixed here.