Skip to content

Commit

Permalink
make sure ww3 restart is read
Browse files Browse the repository at this point in the history
  • Loading branch information
@JessicaMeixner-NOAA
JessicaMeixner-NOAA committed Jan 24, 2025
1 parent 6be9234 commit 72e8c63
Showing 1 changed file with 16 additions and 3 deletions.
19 changes: 16 additions & 3 deletions model/src/w3initmd.F90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -984,9 +984,22 @@ SUBROUTINE W3INIT ( IMOD, IsMulti, FEXT, MDS, MTRACE, ODAT, FLGRD, FLGR2, FLGD,
call extcde (60, msg="required restart file " // trim(fname) // " does not exist")
end if
else
call read_restart('none')
! mapst2 is module variable defined in read of mod_def; maptst is from 2.b above
flcold = .true.
if (restart_from_binary) then
call set_user_timestring(time,user_timestring)
fname = trim(user_restfname)//trim(user_timestring)
inquire(file=trim(fname), exist=exists)
if (exists) then
call w3iors('READ', nds(6), sig(nk), imod, filename=trim(fname))
else
call read_restart('none')
! mapst2 is module variable defined in read of mod_def; maptst is from 2.b above
flcold = .true.
end if
else
call read_restart('none')
! mapst2 is module variable defined in read of mod_def; maptst is from 2.b above
flcold = .true.
end if
end if
else
#endif
Expand Down

14 comments on commit 72e8c63

@DeniseWorthen
Copy link

Choose a reason for hiding this comment

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

I think you might want instead this change. My reasoning here is that you still need an abort if you're asking it to read a fname for an initial run (but it doesn't exist). I think in your case, you could end up cold starting (because the file didn't exist) when that isn't what you intended. The only time you want to read 'none' is an initial run w/ no restart file at all.

diff --git a/model/src/w3initmd.F90 b/model/src/w3initmd.F90
index 2389539f..9547a7f3 100644
--- a/model/src/w3initmd.F90
+++ b/model/src/w3initmd.F90
@@ -985,9 +985,20 @@ CONTAINS
           call extcde (60, msg="required restart file " // trim(fname) // " does not exist")
         end if
       else
-        call read_restart('none')
-        ! mapst2 is module variable defined in read of mod_def; maptst is from 2.b above
-        flcold = .true.
+        if (restart_from_binary) then
+          call set_user_timestring(time,user_timestring)
+          fname = trim(user_restfname)//trim(user_timestring)
+          inquire(file=trim(fname), exist=exists)
+          if (exists) then
+            call w3iors('READ', nds(6), sig(nk), imod, filename=trim(fname))
+          else
+            call extcde (60, msg="required restart file " // trim(fname) // " does not exist")
+          end if
+        else
+          call read_restart('none')
+          ! mapst2 is module variable defined in read of mod_def; maptst is from 2.b above
+          flcold = .true.
+        end if
       end if
     else

@JessicaMeixner-NOAA
Copy link
Owner Author

Choose a reason for hiding this comment

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

That is definitely a concern that you think you start from something but you don't. That's been a cause of issues in the past, but since the waves "can" start from nothing, I think we want some sort of option for that.

I can see future needs to even start with runtype initial but with a netcdf restart - is there some reason we cannot/should not do that? (At least from what I can tell runtype is based on the mediator having a restart file, not necessarily the wave model itself having a variable for that, but perhaps that's a lack of understanding on my end and I could change that setting somewhere else).

Lastly, if we do want to fail when we think we should fail, should there like be a variable for that specifically?

@DeniseWorthen
Copy link

Choose a reason for hiding this comment

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

There is a lot of confusion about what to call various things like coldstart and warmstart, continue and initial.

An initial run can mean that you have "warmstarts" from various models (obtained somehow) but you haven't yet run this particular configuration/date. You do not have a mediator restart file, because you've never coupled & run it before. An initial run can also mean you have basically some sort of IC (like T,S for MOM6 which may in fact come from MOM5 or climatology).

I think what you're asking for is the capability to have "warmstart" (runtype=initial) but from netCDF, not binary. I think that is something we should have. But you need to generate that netCDF file somehow, say by converting an existing binary to netCDF.

@JessicaMeixner-NOAA
Copy link
Owner Author

Choose a reason for hiding this comment

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

Agreed, there is a lot of confusion from the different names.

We could proceed with the binary only, and know that in the future, when we do have things in operations we might have to revisit since we don't have things to test the netcdf option now? Does that sound like a good plan?

@DeniseWorthen
Copy link

Choose a reason for hiding this comment

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

I guess I'm not clear on how/why you'd end up generating binary restarts (in operations) in the future? But if you say so, then yes, that capability would need to be added.

@JessicaMeixner-NOAA
Copy link
Owner Author

Choose a reason for hiding this comment

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

Okay - I have made things confusing - I meant NetCDF restarts in future operations.

How about this:

https://github.com/JessicaMeixner-NOAA/WW3/tree/devufs/addICtoinitresart

@DeniseWorthen
Copy link

Choose a reason for hiding this comment

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

Maybe forgot to push your changes?

@JessicaMeixner-NOAA
Copy link
Owner Author

Choose a reason for hiding this comment

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

I did... they should be there now...

@DeniseWorthen
Copy link

@DeniseWorthen DeniseWorthen commented on 72e8c63 Jan 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm really confused! Are the changes in addICtoinitrestart meant to allow restart_from_binary for run-type=initial?

@JessicaMeixner-NOAA
Copy link
Owner Author

Choose a reason for hiding this comment

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

The changes in addICtoinitrestart are meant to add a capability to read a restart file (binary or netcdf) if it exists, reguardless of runtype. If runtype=continue, then it fails, but if it's initial then it's allowed to start from rest.

I ended up changing the logic b/c one I thought it was a good compromise between failing and not and when I was trying to do an example of allowing for reading a restart (binary or netcf) I realized it was actually simpler to do the logic this way.

That being said, I can walk this back to just a binary file only - and we can add in the netcdf reading later.

@DeniseWorthen
Copy link

Choose a reason for hiding this comment

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

I do think one-step at a time (for now, just allow reading binary when runtype=initial) is best.

@JessicaMeixner-NOAA
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm tempted to let it not fail though since it technically can start from rest, and it should say in the log file (although log.ww3 and not the main log) that it started from rest.

@DeniseWorthen
Copy link

Choose a reason for hiding this comment

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

But the user is expecting that it is starting from a restart (binary) and the model should fail (imho) if what the user intends is not what happens.

@JessicaMeixner-NOAA
Copy link
Owner Author

Choose a reason for hiding this comment

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

Need to test, create ufs-weather-model, etc , but: NOAA-EMC#1359 is there as a draft for now

Please sign in to comment.