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

Update CI #85

Merged
merged 8 commits into from
Feb 2, 2024
Merged

Update CI #85

merged 8 commits into from
Feb 2, 2024

Conversation

zmoon
Copy link
Member

@zmoon zmoon commented Aug 7, 2023

  • moving to the new micromamba Action
  • add more debug flags for GFortran
  • similar for Intel
  • DEBUG=2 option and document (29863dc)

@zmoon
Copy link
Member Author

zmoon commented Aug 7, 2023

@drnimbusrain how do we want to handle FPEs? First one is here (case that hcmref is 0):

--- a/src/canopy_calcs.F90
+++ b/src/canopy_calcs.F90
@@ -78,8 +78,12 @@ SUBROUTINE canopy_calcs
 ! ... calculate wind speed from u and v
                 ubzref   = sqrt((uref**2.0) + (vref**2.0))

-! ... get scaled canopy model profile and sub-canopy layers
-                zhc         = zk/hcmref
+                ! ... get scaled canopy model profile and sub-canopy layers
+                if (hcmref > 0) then
+                    zhc = zk / hcmref
+                else
+                    zhc = 0
+                end if
                 cansublays  = floor(hcmref/modres)

 ! ... check for valid model vegetation types

@drnimbusrain
Copy link
Member

drnimbusrain commented Aug 8, 2023

I agree with your first FPE change here. Do you want to make this (and other) change to your branch as the FPEs come up with your improved debugging options?

@drnimbusrain
Copy link
Member

@zmoon Do you think you will be able to work on this again soon? Thank you!

@zmoon
Copy link
Member Author

zmoon commented Jan 24, 2024

@drnimbusrain I'll try to work on it again soon.

@drnimbusrain
Copy link
Member

drnimbusrain commented Jan 24, 2024 via email

@zmoon
Copy link
Member Author

zmoon commented Jan 24, 2024

That's good, I'll merge that into here before working on it more then.

Initial DEBUG=2 option

some tweaks to flags, including setting error limit 0
for DEBUG=2
with the enhanced debug flags, was complaining about the
past (and/or current) LAI being unitialized in the first
expression of the calculation
@zmoon zmoon marked this pull request as ready for review February 1, 2024 22:49
@zmoon
Copy link
Member Author

zmoon commented Feb 1, 2024

@drnimbusrain some notes:

  • CI debug build still has DEBUG=1 not 2
  • we're currently not testing all code paths, just the default namelist, so there may be other FPEs
  • "array temporary was created" warnings we could alleviate by explicitly creating the temporary arrays. This is mainly for going from an array of floats to assigning to a float member of an array of derived type in the ncf read/write.

@drnimbusrain
Copy link
Member

@zmoon Thank you for working on this, and agree that more comprehensive checks of the different code pathways/options are needed to check for all FPEs. I will try and work on this with your new debug options more thoroughly as we go along with more robust testing. I think that @btang1 @MaggieMarvin @angehung5 could help with this DEBUG=2 testing of the different options for FPEs as well, since they are going to be focusing on different parts of code. For now I approve, and we can improve later.

@drnimbusrain
Copy link
Member

I can also work on the "array temporary was created" issues too. Thanks!

Copy link
Member

@drnimbusrain drnimbusrain left a comment

Choose a reason for hiding this comment

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

Looks OK, but just want to know about the leafage option changes here.

Copy link
Member

Choose a reason for hiding this comment

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

@zmoon Although, what has changed in the leafage option with your PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be nothing. I just moved the calculation under the if statement to avoid FPE related to the LAI being unitialized. Little note in d241bea

@drnimbusrain
Copy link
Member

OK, I will merge then and work with the DEBUG=2 option some to fix some other potential FPEs and the warnings...

@drnimbusrain drnimbusrain merged commit 2d65723 into develop Feb 2, 2024
4 checks passed
@drnimbusrain drnimbusrain deleted the update-ci branch February 2, 2024 15:05
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.

2 participants