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

unable to create a scratch variable in existing scratch group or a package group #17

Closed
jonwoodring opened this issue Feb 10, 2015 · 13 comments
Assignees

Comments

@jonwoodring
Copy link
Member

In Registry_okubo_weiss.xml, I had to create a scratch variable in a separately named group that I called "okuboWeissScratch."

The two places that I tried it, that did not work:

  • in the existing "scratch" group -- this failed at run time in mpas_grid_types.F when it tried to allocate the scratch variable. This was a hard error/crash for MPAS.
  • in the "amOkuboWeiss" package -- this failed at compile time as the compilation told me that I wasn't allowed (I don't remember the exact message)

I don't know if either one of these are bugs or features.

@douglasjacobsen
Copy link
Member

I'm not exactly sure what you mean with this.

I'd have to see examples of the two cases you described as failing to know if they should have, or what the cause was. My guess is that at a minimum the second one is a feature (i.e. you can't add variables within a package, and package variables can't be defined as scratch), but I'd need more information to be sure.

@jonwoodring
Copy link
Member Author

My expectation that was that I could create a scratch variable anywhere, and so the first thing I had tried was creating variables with persistence="scratch" inside of a package. This never compiled (I don't have the compilation logs anymore). I had put these inside the amOkuboWeiss variable definitions in Registry_okubo_weiss.xml

My second attempt was to create scratch variables inside of a <var_struct name="scratch"> inside of Registry_okubo_weiss.xml (i.e., adding new scratch variables to the existing "scratch" container). This one failed at runtime, as there was a backtrace crash when I called mpas_allocate_scratch_field in mpas_grid_types.F.

I fixed it by creating a new <var_struct name="okuboWeissScratch"> set, and defining my scratch variables inside of there.

@douglasjacobsen
Copy link
Member

The only place you can define variables is within a <var_struct ... > ... </var_struct> block. You can create a variable, and attach a package to it, but you can't define variables within a package.

I'd have to see the examples from the second case to know if it's a real issue or just incorrect usage. But I think I can say pretty certainly (if my understanding is correct) that the first is just incorrect usage.

Glad you got it fixed though!

@jonwoodring
Copy link
Member Author

So, putting a scratch variable in <var_struct name="amOkuboWeiss" time_levs="1" packages="amOkuboWeiss"> doesn't work due to the packages tag?

This is where I had put it when I meant packages, not in <package name="amOkuboWeiss"... If you look at Registry_okubu_weiss.xml, the variables that are currently in okuboWeissScratch had been in var_struct name="amOkuboWeiss", just with the persistence="scratch" tag.

That is what didn't compile in the first place, and then I had tried adding them to <var_struct name="scratch"..., but that created the run time crash during mpas_allocate_scratch_field.

@jonwoodring
Copy link
Member Author

And a little more clarification on the second one, this was my assumption:

  • if I create a <var_struct name="scratch"> inside of Registry_foo.xml, those variables will be added to the "scratch" namespace

Is it not possible to add more variables to an existing var_struct namespace if they live in two different registry files, or do var_struct names need to be unique per registry xml?

@douglasjacobsen
Copy link
Member

Oh, yes. I understand more what you were doing now. I think examples like this would benefit from having the registry files that caused the issues.

But you're correct. You can't have the packages tag on a var_struct if the var_struct contains scratch variables. I can add something for us to think about as far as changing this (i.e. a scratch variable ignores packages....) but it can get confusing to deal with quickly, so it's generally easier to just make a hard rule like this.

As far as the second issue (defining an additional var_struct with the name scratch), that does work fine. The structures get merged during build time, and the resulting structure contains all fields defined in both of them. I'd have to see what Registry.xml file you tried, what code you tried to run (i.e. with the mpas_allocate_scratch_field), and what the resulting error was to be able to debug what was going on though.

@jonwoodring
Copy link
Member Author

Actually, that’s pretty easy to test. If you take my current branch, and change “okuboWeissScratch” to “scratch” in Registry_okubo_weiss.F, it should crash at the first mpas_allocate_scratch_field call in mpas_ocn_okubo_weiss.F.

I verified this by allocating an existing field that exists in “scratch” in Registry.xml. That worked just fine.

The scratch variables that I defined in “scratch” var_struct in Registry_okubo_weiss.xml, made it crash at that call.

-Jon

From: Doug Jacobsen <notifications@github.commailto:notifications@github.com>
Reply-To: MPAS-Dev/MPAS-Release <reply@reply.github.commailto:reply@reply.github.com>
Date: Tuesday, February 10, 2015 at 6:37 PM
To: MPAS-Dev/MPAS-Release <MPAS-Release@noreply.github.commailto:MPAS-Release@noreply.github.com>
Cc: Jon Woodring <woodring@lanl.govmailto:woodring@lanl.gov>
Subject: Re: [MPAS-Release] unable to create a scratch variable in existing scratch group or a package group (#17)

Oh, yes. I understand more what you were doing now. I think examples like this would benefit from having the registry files that caused the issues.

But you're correct. You can't have the packages tag on a var_struct if the var_struct contains scratch variables. I can add something for us to think about as far as changing this (i.e. a scratch variable ignores packages....) but it can get confusing to deal with quickly, so it's generally easier to just make a hard rule like this.

As far as the second issue (defining an additional var_struct with the name scratch), that does work fine. The structures get merged during build time, and the resulting structure contains all fields defined in both of them. I'd have to see what Registry.xml file you tried, what code you tried to run (i.e. with the mpas_allocate_scratch_field), and what the resulting error was to be able to debug what was going on though.


Reply to this email directly or view it on GitHubhttps://github.com/MPAS-Dev/MPAS-Release/issues/17#issuecomment-73820872.

@douglasjacobsen
Copy link
Member

So, were you making the entire var_struct scratch?

i.e. changing:
<var_struct name="okuboWeissScratch" time_levs="1">

to

<var_struct name="okuboWeissScratch" time_levs="1" persistence="scratch">

?

If that's the case, then currently framework doesn't allow var_structs to be marked as scratch. Only <var and <var_array blocks can be marked as scratch. But that just causes all variables within the var_struct to be marked as persistent instead of scratch (assuming they don't have persistence="scratch" themselves). What mesh were you running on when you had that issue?

@jonwoodring
Copy link
Member Author

No sorry, the name was "scratch," i.e., <var_struct name="scratch"...

The variables inside were persistence="scratch" I'll make a branch or a gist tomorrow. I think that will help.

Jon


From: Doug Jacobsen
Sent: Tuesday, February 10, 2015 7:04:11 PM
To: MPAS-Dev/MPAS-Release
Cc: Woodring, Jonathan Lee
Subject: Re: [MPAS-Release] unable to create a scratch variable in existing scratch group or a package group (#17)

So, were you making the entire var_struct scratch?

i.e. changing:
<var_struct name="okuboWeissScratch" time_levs="1">

to

<var_struct name="okuboWeissScratch" time_levs="1" persistence="scratch">

?

If that's the case, then currently framework doesn't allow var_structs to be marked as scratch. Only <var and <var_array blocks can be marked as scratch. But that just causes all variables within the var_struct to be marked as persistent instead of scratch (assuming they don't have persistence="scratch" themselves). What mesh were you running on when you had that issue?


Reply to this email directly or view it on GitHubhttps://github.com/MPAS-Dev/MPAS-Release/issues/17#issuecomment-73823182.

@douglasjacobsen
Copy link
Member

Sure. I'd like to see a branch or a gist to test it out. Just a note, if you just change the name to scratch, it won't work in your code, because you're attempting to get the fields from a different pool (that doesn't exist anymore).

@jonwoodring
Copy link
Member Author

Oh good point. I'll make a patch for both cases.

Jon


From: Doug Jacobsen
Sent: Tuesday, February 10, 2015 7:39:23 PM
To: MPAS-Dev/MPAS-Release
Cc: Woodring, Jonathan Lee
Subject: Re: [MPAS-Release] unable to create a scratch variable in existing scratch group or a package group (#17)

Sure. I'd like to see a branch or a gist to test it out. Just a note, if you just change the name to scratch, it won't work in your code, because you're attempting to get the fields from a different pool (that doesn't exist anymore).


Reply to this email directly or view it on GitHubhttps://github.com/MPAS-Dev/MPAS-Release/issues/17#issuecomment-73825754.

@jonwoodring
Copy link
Member Author

Hmm. OK.

So, doing the first case, putting scratch variables inside of var_struct name="amOkuboWeiss" failed with this message:

Reading registry file from standard input
ERROR: Packages attribute not allowed on scratch variable thresholdedOkuboWeiss in var_struct amOkuboWeiss.

For the second case, putting the variables inside of "scratch," it's working now. I'm finding it to work now at least with 16 cores. I'm scaling it up now to see if it crashes at a higher core count. I will let you know if it works.

Though, when I originally tried it, it was based off the 3.2 and I hadn't incorporated 3.3 hotfixes yet. Would there have been something that fixed it since 3.2 to 3.3?

@douglasjacobsen
Copy link
Member

@jonwoodring Thanks for testing this. It sounds like those are both expected results from the tests. I can't think of anything that would have changed this between 3.2 and 3.3, but it's possible. I don't know that it's worth going back and trying to find the issue since it appears fixed, so I'm going to mark this as closed.

Let me know if you're like me to open it and look into the problem more.

Thanks again for your help!

mgduda added a commit to mgduda/MPAS-Model that referenced this issue Aug 5, 2020
…routines

This merge replaces the calls to the mpas_pool_get_array_gpu function
with calls to mpas_pool_get_array for the routines solve_diagnostics and
init_coupled_diagnostics. The gpu calls have !$acc data copyin directives
and they transfer the variables on to gpu. The two functions solve_diagnostics
and init_coupled_diagnostics are also called outside the time integration.
The data copying was corrupting the GPU data later in the timestep, resulting
in NaNs.

* removal_of_getarray_gpu_call:
  Removal of mpas_get_array_gpu calls
byoung-joo pushed a commit to byoung-joo/MPAS-Model that referenced this issue Apr 6, 2021
DESCRIPTION OF CHANGES:
Remove warnings (mentioned in ISSUE JCSDA-internal#17) when using the 2-stream initial state in an mpas_atmosphere forecast.

LIST OF MODIFIED FILES:
M src/core_atmosphere/Registry.xml

ISSUES:
Closes MPAS-Dev#17 

TESTS CONDUCTED: 
Tested on Cheyenne. No impact.
climbfuji pushed a commit to climbfuji/MPAS-Model that referenced this issue Aug 25, 2022
DESCRIPTION OF CHANGES:
Remove warnings (mentioned in ISSUE JCSDA-internal#17) when using the 2-stream initial state in an mpas_atmosphere forecast.

LIST OF MODIFIED FILES:
M src/core_atmosphere/Registry.xml

ISSUES:
Closes MPAS-Dev#17 

TESTS CONDUCTED: 
Tested on Cheyenne. No impact.
amrapallig pushed a commit to amrapallig/MPAS-Model that referenced this issue Dec 9, 2022
Generalize pressure constants and make consistent with documentation
LarissaReames-NOAA pushed a commit to LarissaReames-NOAA/MPAS-Model that referenced this issue Jul 8, 2024
* Add support for RAP initial conditions

* Add cycled aerosol information to ICs and LBCs

* patch for dp=0 problem + some cleanup

---------

Co-authored-by: Anders Jensen <anders.jensen@noaa.gov>
barlage pushed a commit to barlage/MPAS-Model that referenced this issue Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants