Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Some collapse options should be disabled with FATES #1780

Closed
ekluzek opened this issue Jun 10, 2022 · 3 comments
Closed

Some collapse options should be disabled with FATES #1780

ekluzek opened this issue Jun 10, 2022 · 3 comments
Labels
enhancement new capability or improved behavior of existing capability

Comments

@ekluzek
Copy link
Collaborator

ekluzek commented Jun 10, 2022

@rgknox points this out in his most recent PR #1766. The collapse options that work on landunit should work provided FATES is using create_crop_landunit=F and 16-PFT datasets are being read and you don't have FATES managing crop landunits. But, "n_dom_pfts" is incompatible with FATES, except maybe for FATES-SP mode. I think we should disable it for all FATES modes though. "toosmall_soil" is also problematic and likely should be disabled.

When FATES can manage crop landunits, the "toosmall_crop" might be problematic. But, for the case when we run FATES for natural veg and BGC-CROP for CFT's, "toosmall_crop" should be fine.

It also a question on if we should test FATES with the collapse options. I don't think this is a use case that would at all be common so I'm thinking it shouldn't be.

@ekluzek ekluzek added enhancement new capability or improved behavior of existing capability type: -discussion next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Jun 10, 2022
@ekluzek ekluzek removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Jun 30, 2022
@ekluzek
Copy link
Collaborator Author

ekluzek commented Jun 30, 2022

We had some discussion about this last week. We figure we should allow options that should work with FATES to exist, but won't add explicit testing for it, as we don't think this is a normal way of working. Perhaps if people start to use it down the line, we'll need to add testing.

We did disable the use of n_dom_pfts with FATES in #1766, but this should likely also going into the FORTRAN code and apply to the other things pointed out above as well.

@glemieux
Copy link
Collaborator

glemieux commented Dec 15, 2023

Just a quick note that this came up in testing fates landuse v1; see #2076 (comment). In summary, collapse_to_dominant was being passed numpft bounds to set the dummy weight argument bounds inside the procedure, even though surfpft bounds are technically used to allocate the actual input weight argument. Currently these two bounds will only ever diverge if fates is used. The short term fix proposed and implemented in the above PR is to simply align the input argument bounds with the allocation argument bounds.

I should note that this appears to only be an issue for the nag compiler. This passed through the gnu compiler on derecho.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Aug 14, 2024

I'm converting this to a discussion. If we work on this we should map out exactly what needs to be done.

@ESCOMP ESCOMP locked and limited conversation to collaborators Aug 14, 2024
@ekluzek ekluzek converted this issue into discussion #2690 Aug 14, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement new capability or improved behavior of existing capability
Projects
Status: Done
Development

No branches or pull requests

2 participants