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

Outputted Time Variable is Inconsistent, affecting input to mizuroute #528

Open
KyleKlenk opened this issue Jul 4, 2023 · 2 comments
Open

Comments

@KyleKlenk
Copy link
Contributor

Bug Reports

  • Commit: fa9adf8

  • GNU Fortran (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0

  • Ubuntu 20.0.4

  • Description of settings: All settings are unmodified after being produced with the code here: https://github.com/CH-Earth/CWARHM for the Great Slave Lake area.

  • Summary:
    The time value outputted by SUMMA is not a whole number and does not have consistent spacing. This is causing issues when using the output of a SUMMA run to be input for mizuroute.

Here is an example of the time variable output I get from running SUMMA:
567993600.00000000, 567997199.99998660, 568000800.00001340, 568004400.00000000, 568007999.99998660, 568011600.00001340 ....

I have looked in the code and have traced this behaviour to: read_force.f90:194

do iGRU=1,size(gru_struc)
  do iHRU=1,gru_struc(iGRU)%hruCount
   forcStruct%gru(iGRU)%hru(iHRU)%var(iLookFORCE%time) = (currentJulday-refJulday)*secprday
  end do  ! looping through HRUs
 end do  ! looping through GRUs

Here currentJulDay is represented as a decimal and causes the resulting time value to contain a decimal as well.

Is this expected behaviour? It looks like seconds since is the only format the time variable is output with as per:

forc_meta(iLookFORCE%time)                  = var_info('time'    , 'time since time reference'                         , 'seconds since 1990-1-1 0:0:0.0 -0:00', get_ixVarType('scalarv'), iMissVec, iMissVec, .false.)

Is it okay to add a fix that rounds the number of seconds to the nearest whole number? It looks like this can be done in read_force.f90:194 or when writing the output at modelwrite.f90:210.

Or is there a way to configure SUMMA to avoid the time variable output behaviour I am experiencing?

@martynpclark
Copy link
Collaborator

Hi Kyle, as we discussed over slack, the rounding fix should test the assumption that the time units are seconds. If not, and someone changes the time units (e.g., to days), then the rounding will no longer be valid and the error may be difficult to find.

@KyleKlenk
Copy link
Contributor Author

KyleKlenk commented Jul 5, 2023

Would it make sense to add an additional variable to globalData.f90 and case statements set for this value in the output? or would an additional variable to the forcStruct make more sense?

I also think maybe a warning message at the start of the code may be helpful for this? Just testing against a variable might still be tricky to find if it is unknown this setting/variable exists. I am just thinking for my suggestion about putting in globalData.f90, as I would easily overlook this setting myself.

I don't see any existing variables that we could use to test against currently. If there is one I will use it.

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

No branches or pull requests

2 participants