Migrate time stamp control from namelist to the meteorological driver header #413
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…roduces variable met_avgtype in the meteorological driver header. This change makes the meteorological driver creator (as opposed to the end user) responsible for setting the time stamp standard that was used for the meteorological driver. It also allows for mixing meteorological drivers with different standards (not that this is a good idea). This pull request also changes the path for the CI tests. The original links pointed to a specific user. The files are now hosted in EDModel organisation, which makes it easier for tracking and maintaining it.
…ces for the test suite. Somehow the CI tests were crashing due to a "merge" command in the inventory initialisation. Switching the merge with an equivalent "where" construction seems to fix it. This could be a compiler issue, but if the problem pops up again, we may want to deprecate merge in the code and go with where instead.
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request removes variable
NL%IMETAVGfrom the ED2 namelist (ED2IN), and introduces variablemet_avgtypein the meteorological driver header. The rationale for this change is to keep all settings for meteorological drivers in the same file.This pull request also changes the path for the CI tests. The original links pointed to a specific user. The files are now hosted in EDModel organisation, which makes it easier for tracking and maintaining it.
Collaborators
Motivation and Context
This pull request should significantly reduce the risk of model crashes due to users not setting variable
NL%IMETAVGcorrectly, one of the most common causes of the dreadedIFLAG1crash error. The new flag informing how to interpret the time stamp in the meteorological drivers will be placed together with the other meteorological driver settings, which indirectly makes the meteorological driver creator responsible for setting this flag correctly, instead of the end user.Another feature is that it is now possible to mix meteorological drivers with different time stamp settings, because
met_avgtypemust be defined for each driver source (not that this is necessarily a good idea).Types of changes
Important. This code change will require everyone updating their meteorological driver headers. However, this will be a one and done fix that will likely make the code less susceptible to crashes due to inconsistent settings.
Checklist:
I will update the documentation once this pull request is merged.
Testing :