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

remove compiling warning #1623

Merged
merged 7 commits into from
May 8, 2024

Conversation

jiandewang
Copy link
Collaborator

This mini PR address the compiling warning removing request by NCEP Central Operational branch. Thanks for @marshallward's work.

jiandewang and others added 5 commits March 4, 2024 14:33
…n-20240228

update to main 20240228 commit
This patch clears out many errors detected by Intel Fortran.

Most are false positives from stub functions which would normally be
replaced in production and report unset output.  These variables are now
assigned dummy values in order to pacify the compiler.

The `stat` function in POSIX was incorrectly passing its `buf` object to
the C `stat` function as `intent(in)`, causing the compiler to believe
that the contents were unset.  Oddly, this was already working
correctly, and perhaps warrants further investigation, but it has now
been correctly set to `intent(inout)`.

The `ppoly_*` variables in `check_reconstruction_1d` appear to have been
incorrectly declared as `out`, when they are clearly used as `in` to
validate the values.  This has been corrected.

`register_diag_field` in the ice shelf diag manager was incorrectly
declared and the function appeared to return nothing.  Perhaps this
function was not used for anything.

An IO statement in MOM_open_boundary had a syntax error; this has been
fixed.

`get_dataset` returns a `dataset_type`, so some compilers expect the
stub function to also return a valid `dataset`.  Since the stub
`dataset_type` contains no fields, any locally declared instance should
be sufficient as a return value.
…n-20240401

update to MOM6 main repo 20240401 commit
Copy link
Collaborator

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

I proudly approve my own PR 🤣

Copy link
Collaborator

@kshedstrom kshedstrom left a comment

Choose a reason for hiding this comment

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

I approve this PR

@klindsay28
Copy link
Contributor

The initializations to -1_real32 and -1_real64 seem peculiar to me. The real32 and real64 kind type parameters are for real values but they are being applied to -1, which by itself is an integer. If you really want to specify the real kind on the right-hand side of these initializations, I would expect to see -1._real32 and 1._real64. Otherwise, you could just use -1 and let the compiler convert the integer to the type of the left-hand side.

@marshallward
Copy link
Collaborator

@klindsay28 Thanks for catching this. I agree: -1_real64 would be interpreted as -1_kind where kind is the int kind that happens to equal real64.

As for whether to let the compiler convert -1.0 (or even -1) to real32/real64: 🤷 I reckon it's better to write what it is, but I don't really care. In retrospect, I think NaN might be a better default value anyway? These functions should never even be called.

The default values for the database transfer functions were incorrectly
assiged as integer literals, recast to types using real32/64 but
actually corresponding to whatever integer kind equals real32/64.

We now simply assign it a literal value of -1. and rely on the compiler
to handle the recasting.

Although none of these functions were intended to be used, and -1 would
probably be eventually cast into an appropriate real type, it is better
to get this correct.

Thanks to Keith Lindsay for suggesting this change.
@marshallward
Copy link
Collaborator

@jiandewang @klindsay28 I've submitted a PR to this branch which changes the -1_real32 and -1_real64 values to -1..

Replace db array default values with real literals
@jiandewang
Copy link
Collaborator Author

@jiandewang @klindsay28 I've submitted a PR to this branch which changes the -1_real32 and -1_real64 values to -1..

just merged. Thanks

@jiandewang jiandewang closed this Apr 26, 2024
@jiandewang
Copy link
Collaborator Author

somehow I closed this by mistake. Re-open it

@jiandewang jiandewang reopened this May 2, 2024
@jiandewang
Copy link
Collaborator Author

@alperaltuntas and @abozec can you review it ?

Copy link
Collaborator

@abozec abozec left a comment

Choose a reason for hiding this comment

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

COAPS approves.

@jiandewang
Copy link
Collaborator Author

@alperaltuntas can you review this PR ? (I guess it maybe bypassed your radar)

@gustavo-marques
Copy link
Collaborator

NCAR approves this PR.

@jiandewang
Copy link
Collaborator Author

thanks for everybody's reviewing. I am going to merge it

@jiandewang jiandewang merged commit 129e1bd into mom-ocean:main May 8, 2024
18 of 20 checks passed
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.

7 participants