Isca icon indicating copy to clipboard operation
Isca copied to clipboard

Added signature to mosaic_util header file.

Open aramirezreyes opened this issue 4 years ago • 3 comments

Hello:

I am trying to compile Isca to run on the Cori Computer at NERSC (I am a PhD student at UC Davis working with Dr. Da Yang)

When trying to compile using gcc, I get a warning that the function isHeaderNode() is not defined when trying to compile mosaic. It is true, and the function is defined at mosaic_util.c but it does not appear on the corresponding header file.

This commits adds the corresponding signature to mosaic_util.h and allows to continue the compilation.

Apologies if it is an inappropriate commit.

aramirezreyes avatar Mar 20 '20 19:03 aramirezreyes

Thanks. Certainly not inappropriate. We’ll take a look. But it may take a bit longer than normal.

gkvallis avatar Mar 21 '20 06:03 gkvallis

I think this looks good overall, it's nice to have more users and environment options in the code!

As a rule, our main criteria for code changes, aside from that they achieve the desired outcome, is that they don't affect the results of previous work unless there was a bug. I think there might be a couple of files in your pull request that could affect current test cases/model running outside NERSC:

  • held_suarez_test_case.py : you've commented out the compile step, is this edit meant to be included in this pull request? I think this will cause problems with our trip-tests.
  • templates/run.sh : edits here look like they change a current default?

I'll run trip-tests on this request sometime over the next few days to check the other edits don't have any unexpected side-effects, but at a glance I can't see that they should.

Hope you're having fun with the model, thanks for contributing!

Ruth

RuthG avatar Apr 29 '20 16:04 RuthG

Hi Ruth. Thanks for checking this out. Now I see that the PR is quite dirty, I thought I had included only one minor change. Will make a second attempt soon.

aramirezreyes avatar May 05 '20 21:05 aramirezreyes