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

Allow for read of tlat, tlon, anglet with popgrid #463

Merged
merged 22 commits into from
Jun 23, 2020

Conversation

TillRasmussen
Copy link
Contributor

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

  • Short (1 sentence) summary of your PR:
    Eliminate warning due to print of iblk when not in a loop
  • Developer(s):
    Till Rasmussen, DMI
  • Suggest PR reviewers from list in the column to the right.
    @apcraig
  • Please copy the PR test results link or provide a summary of testing completed below.
    ENTER INFORMATION HERE
  • How much do the PR code changes differ from the unmodified code?
    • [x ] bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • [x ] No
  • Does this PR add any new test cases?
    • Yes
    • [x ] No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • [x ] No, does the documentation need to be updated at a later time?
      • Yes
      • [ x] No
  • Please provide any additional information or relevant details below:

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

Thanks @TillRasmussen
Most of the changes in ice_grid.F90 are not needed, although I don't mind them except that 'call Tlatlon' is not longer horizontally aligned. The new blank line and line 477 shouldn't be there, and having the blank lines in lines 809 and following are helpful when reading the code. But these are all just picky comments and okay........

@TillRasmussen
Copy link
Contributor Author

The blank spaces is a result of some test I have been doing in order to read anglet, tlon and tlat instead of calculating these. This included adding some if else around the Tlonlat function..

@eclare108213
Copy link
Contributor

@TillRasmussen is this PR ready to merge or are you still working on it?
Is the call to ice_barrier necessary?

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

I recommend getting rid of the ice_barrier as well. Otherwise, looks fine.

call ice_read_global_nc(fid_grid,1,fieldname,work_g1,diag)
call scatter_global(TLAT, work_g1, master_task, distrb_info, &
field_loc_center, field_type_scalar)
endif
Copy link
Contributor

Choose a reason for hiding this comment

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

I looks like there might be a slight indentation problem in this if block. Can you check that there are no "tabs".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thnk that this has been solved

@apcraig apcraig changed the title bug fix Allow for read of tlat, tlon, anglet with popgrid Jun 16, 2020
@TillRasmussen
Copy link
Contributor Author

TillRasmussen commented Jun 21, 2020

This pull request has been updated. Editiing is done if test are complete
1/ To fix bug in ice_Transport driver .F90 (original pull request)
2/ Fix bug when cpp flag NEMO_IN_CICE is enabled. ice_step_mod.F90 uses variable raice without declaring.
3/ Allow for reading angle, tlon tlat if angle exist in netcdf grid file. If these are read they will not be calculated. This will cause differences when either of these three variables are used or dumped to output. If angle do not exist the routine will calculate angle, tlon and tlat as previous.
ice_barrier has been removed

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

It looks like something happened with Icepack and you'll be pulling in a different version. I think this needs to be fixed.

@TillRasmussen
Copy link
Contributor Author

My icepack fork is now updated

@apcraig apcraig merged commit 3fedc78 into CICE-Consortium:master Jun 23, 2020
@apcraig
Copy link
Contributor

apcraig commented Jun 24, 2020

I ran a full test suite last night and everything looks good, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash#3fedc78b1b012f8bd6c0fde7c1b41e79b9db63a9

phil-blain added a commit to phil-blain/CICE that referenced this pull request Aug 23, 2022
In 3fedc78 (Allow for read of tlat, tlon, anglet with popgrid (CICE-Consortium#463),
2020-06-24), ice_grid::init_grid2 was changed so that ice_grid::Tlatlon,
which computes the TLAT and TLON arrays from ULAT and ULON, is only
called if the private module variable 'l_readCenter' is false.

The idea is that if the grid file contains a variable 'anglet', then it
is assumed that it also contains variables 'tlon' and 'tlat', and so
these fields are read directly instead of being computed.

This logic, however, was only implemented in ice_grid::popgrid_nc, which
sets 'l_readCenter' depending on the presence or absence of 'anlglet' in
the grid file. This means that if 'popgrid_nc' is not called (for
example with "grid_format='bin'", in which case init_grid2 calls
'popgrid' and not 'popgrid_nc'), then 'l_readCenter' is used
uninitialized in init_grid2, and so it's possible that 'Tlatlon' is not
called, if 'l_readCenter' happens to be initialized to true.

This in turns leads to 'TLAT' and 'TLON' being uninitialized, and the
code failing when accessing these arrays if compiling with NaN
initialization.

Fix this by initializing 'l_readCenter' at the beginning of init_grid2,
such that it is initialized for all choices of 'grid_format' and
'grid_type'. Remove the initialization in 'popgrid_nc'.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Aug 23, 2022
In 3fedc78 (Allow for read of tlat, tlon, anglet with popgrid (CICE-Consortium#463),
2020-06-24), ice_grid::init_grid2 was changed so that ice_grid::Tlatlon,
which computes the TLAT and TLON arrays from ULAT and ULON, is only
called if the private module variable 'l_readCenter' is false.

The idea is that if the grid file contains a variable 'anglet', then it
is assumed that it also contains variables 'tlon' and 'tlat', and so
these fields are read directly instead of being computed.

This logic, however, was only implemented in ice_grid::popgrid_nc, which
sets 'l_readCenter' depending on the presence or absence of 'anglet' in
the grid file. This means that if 'popgrid_nc' is not called (for
example with "grid_format='bin'", in which case init_grid2 calls
'popgrid' and not 'popgrid_nc'), then 'l_readCenter' is used
uninitialized in init_grid2, and so it's possible that 'Tlatlon' is not
called, if 'l_readCenter' happens to be initialized to true.

This in turns leads to 'TLAT' and 'TLON' being uninitialized, and the
code failing when accessing these arrays if compiling with NaN
initialization.

Fix this by initializing 'l_readCenter' at the beginning of init_grid2,
such that it is initialized for all choices of 'grid_format' and
'grid_type'. Remove the initialization in 'popgrid_nc'.
apcraig pushed a commit that referenced this pull request Aug 24, 2022
In 3fedc78 (Allow for read of tlat, tlon, anglet with popgrid (#463),
2020-06-24), ice_grid::init_grid2 was changed so that ice_grid::Tlatlon,
which computes the TLAT and TLON arrays from ULAT and ULON, is only
called if the private module variable 'l_readCenter' is false.

The idea is that if the grid file contains a variable 'anglet', then it
is assumed that it also contains variables 'tlon' and 'tlat', and so
these fields are read directly instead of being computed.

This logic, however, was only implemented in ice_grid::popgrid_nc, which
sets 'l_readCenter' depending on the presence or absence of 'anglet' in
the grid file. This means that if 'popgrid_nc' is not called (for
example with "grid_format='bin'", in which case init_grid2 calls
'popgrid' and not 'popgrid_nc'), then 'l_readCenter' is used
uninitialized in init_grid2, and so it's possible that 'Tlatlon' is not
called, if 'l_readCenter' happens to be initialized to true.

This in turns leads to 'TLAT' and 'TLON' being uninitialized, and the
code failing when accessing these arrays if compiling with NaN
initialization.

Fix this by initializing 'l_readCenter' at the beginning of init_grid2,
such that it is initialized for all choices of 'grid_format' and
'grid_type'. Remove the initialization in 'popgrid_nc'.
dabail10 pushed a commit to ESCOMP/CICE that referenced this pull request Oct 4, 2022
…um#758)

In 3fedc78 (Allow for read of tlat, tlon, anglet with popgrid (CICE-Consortium#463),
2020-06-24), ice_grid::init_grid2 was changed so that ice_grid::Tlatlon,
which computes the TLAT and TLON arrays from ULAT and ULON, is only
called if the private module variable 'l_readCenter' is false.

The idea is that if the grid file contains a variable 'anglet', then it
is assumed that it also contains variables 'tlon' and 'tlat', and so
these fields are read directly instead of being computed.

This logic, however, was only implemented in ice_grid::popgrid_nc, which
sets 'l_readCenter' depending on the presence or absence of 'anglet' in
the grid file. This means that if 'popgrid_nc' is not called (for
example with "grid_format='bin'", in which case init_grid2 calls
'popgrid' and not 'popgrid_nc'), then 'l_readCenter' is used
uninitialized in init_grid2, and so it's possible that 'Tlatlon' is not
called, if 'l_readCenter' happens to be initialized to true.

This in turns leads to 'TLAT' and 'TLON' being uninitialized, and the
code failing when accessing these arrays if compiling with NaN
initialization.

Fix this by initializing 'l_readCenter' at the beginning of init_grid2,
such that it is initialized for all choices of 'grid_format' and
'grid_type'. Remove the initialization in 'popgrid_nc'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants