Implement DIC and ALK sensors to particles

[bradyrx]

@pwolfram @maltrud

Adds functionality for BGC sensors, beginning with DIC and ALK. Also updates variable naming convention for temperature and salinity to camel case with "particle" as the prefix.

Please double check the conditionals I threw in there. It looks like we need to check that the ocean ecosystem is turned on in a few spots before pulling values for e.g. DIC and ALK.

[pwolfram]

@bradyrx, turning on salinity and temperature work with BGC parameters off, correct? Does the issue wit the segfault occur when the BGC parameters are turned on?

[pwolfram]

Also, do you mind posting an example of the seg-fault that you are getting? I would think if temperature and salinity work it shouldn't be too hard to get the others to work. Also, have you tried a build with DEBUG=true?

[bradyrx]

@bradyrx, turning on salinity and temperature work with BGC parameters off, correct? Does the issue wit the segfault occur when the BGC parameters are turned on?

No, this is the most egregious problem. I've tried running this as a g-case with BGC (i.e. GMPAS-OECO-ODMS-IAF) with only the physical sensors on and there's still a segfault. So the existence of the BGC conditionals break this.

I don't think I've tried my code base as a physics-only g-case (i.e. GMPAS-IAF) with only physical sensors on. I'll send that off now to see if that works.

So for clarity, with the implementation of BGC sensor code, it segfaults with any sensors on in a BGC-active case.

[bradyrx]

Also, do you mind posting an example of the seg-fault that you are getting? I would think if temperature and salinity work it shouldn't be too hard to get the others to work. Also, have you tried a build with DEBUG=true?

@pwolfram,

Here are some case directories that segfaulted. I think all of them were built with DEBUG=true.

BGC Sensor code, physical sensors only turned on

/lustre/scratch3/turquoise/rileybrady/ACME/cases/GMPAS-OECO-ODMS-IAF.T62_oEC60to30v3.bgcSensors07_wolf_physicalsensorsonly

BGC Sensor code, all sensors (including DIC and ALK) turned on

/lustre/scratch3/turquoise/rileybrady/ACME/cases/GMPAS-OECO-ODMS-IAF.T62_oEC60to30v3.bgcSensors03_wolf

[bradyrx]

@pwolfram,

Note that the physics-only G-case (GMPAS-IAF) with only temp/salinity sensors turned on seg-faulted as well:

/lustre/scratch3/turquoise/rileybrady/ACME/cases/GMPAS-IAF.T62_oEC60to30v3.bgcSensors.activeOnly/run

[pwolfram]

@bradyrx, I think I found the bug as detailed in my review comments above. Please let me know what you find out. If you already tested pwolfram:particlePassiveFloatVerticalTreatmentFix and there was a seg-fault with temperature and salinity particles on then we actually have >= 2 bugs and I've only found one of them as detailed in the review.

Note, I have NOT updated this PR and will let you fix that code to keep your authorship of the new lines.

[pwolfram]

@bradyrx, if you have my fork and cherry-pick 9c620c18278f43e574a2ffc0771d7a8f6bdd2dab I think you'll find this helps and takes care of the particleTemperature vs temperature (and salinity variant) issue.

[pwolfram]

This is also on branch_fix_riley, please make sure that this works for you following a rebase on particlePassiveFloatVerticalTreatmentFix.

[bradyrx]

@pwolfram (cc @maltrud),

BGC sensors now online! (see below for particleDIC) Thanks for the catch. I actually eyed that line a few different times as a potential error but think I didn't understand the function call. The thought was that in:

call mpas_pool_get_array(particle % haloDataPool, 'temperature', particleTemperature)

The quoted 'temperature' referenced the MPAS variable name and that it was getting assigned to our variable name of particleTemperature.

Fix is implemented in this PR for DIC, ALK, temperature, and salinity. Going to implement a few other BGC sensor options now.

[pwolfram]

I did add a commit with this fix— did you see it? On Oct 15, 2018, at 10:11 AM, Riley Brady notifications@github.com<mailto:notifications@github.com> wrote:

@bradyrx commented on this pull request.

---

In testing_and_setup/compass/utility_scripts/LIGHTparticles/streams.oceanhttps://github.com/pwolfram/MPAS-Model/pull/2#discussion_r225227268:

@@ -68,7 +68,7 @@

<stream name="lagrPartTrackOutput" runtime_format="single_file"

• filename_interval="01-00-00_00:00:00"
• filename_interval="00-01-00_00:00:00" clobber_mode="truncate" reference_time="0001-01-01_00:00:00" output_interval="00-00-02_00:00:00"

Yes, will add streams suggestions here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/pwolfram/MPAS-Model/pull/2#discussion_r225227268, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEGMrcD22F1t6Jf3eWWwMG_11gj21xpqks5ulLPLgaJpZM4V9BvG.

[bradyrx]

@pwolfram , didn't catch it in this thread. Noted.

[bradyrx]

Tasks ahead:

• [x] Add sensor capability for major nutrients (index list here)
• [x] Check https://github.com/pwolfram/MPAS-Model/issues/7

[bradyrx]

@pwolfram @maltrud

BGC sensors are ready to go. Just ran a 60to30 test case with all passive tracers and it ran fine. As of now, there is capability for DIC, ALK, PO4, NO3, SiO3, NH4, Fe, and O2 sensors, although they need to be manually turned on in the registry. I am happy to add other passive tracers (e.g. phyto groups) at any point, but these should do for now.

Run can be found here:

/lustre/scratch3/turquoise/rileybrady/ACME/cases/GMPAS-OECO-ODMS-IAF.T62_oEC60to30v3.bgcSensors.allOn/run

I'll check the index levels and then this should be good to merge.

[pwolfram]

@bradyrx, did you end up doing some cherry-picks from the head branch post-facto? It looks like my merge commits from https://github.com/pwolfram/MPAS-Model/commits/particlePassiveFloatVerticalTreatmentFix aren't represented here and I'm thinking we are potentially double-counted some of the commits, which we don't want to do. I may have to clean this up during a merge but it shouldn't be a big deal as long as the code at the end is identical to your working branch you've been using.

[bradyrx]

@pwolfram,

I just rebased onto your branch particlePassiveFloatVerticalTreatmentFix, so everything from there is represented in this PR to my knowledge. I.e., the latest commit of yours on there is be19e757eedcc9bd5325370ec5a11091a0ac6449.

I wasn't sure how to bring in commits from branch_fix_riley since it was a separate branch, so I just grabbed 82156c2c882a467ada476f7bcef27ac7f78a74c1 and 9c620c18278f43e574a2ffc0771d7a8f6bdd2dab from there.

[pwolfram]

@bradyrx, do we still needs this PR? I'm thinking it has been subsumed by #21. I'm unclear what is in this PR but missing in the others. Any clarity you can provide here is greatly appreciated.

[bradyrx]

I think we should be fine closing this and going forth with https://github.com/pwolfram/MPAS-Model/pull/21.

What might be most straightforward is to merge https://github.com/pwolfram/MPAS-Model/pull/21 following your testing, and then I'll open a separate PR to tack on horizontal interp. capabilities for the remaining BGC tracers (e.g. nitrate, phosphate). Does that sound good?

[pwolfram]

Perfect, thanks @bradyrx! We can reopen too if needed.