-
Notifications
You must be signed in to change notification settings - Fork 234
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
Sda dongchen #3052
Sda dongchen #3052
Conversation
|
||
# host specific setup | ||
@HOST_SETUP@ | ||
@CDO_SETUP@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide an example of what @CDO_SETUP@ should look like? is that the module command? the path? both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also dont see a space in Create_Multi_settings.R where you put in the CDO options so that they get written to the XML. Does that info not go into the XML file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, it should look like "module load cdo/2.0.6", it's SCC specific setup btw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, I am adding it right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@serbinsh already updated the create_multi_settings script and it's now having the cdosetup in the host section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dongchen, I see two things that need to happen before I can approve this PR:
- The new tags you've added need to be added to the overall PEcAn documentation, including examples like the one in the thread above so that folks know how to use these new tag
- These tags need to be added to the SIPNET job template, not the generic model template, for them to work. Furthermore, they need to be there in the default job template, not a geo specific one, otherwise your code will fail. Indeed, the problem you've identified and fixed is in no way specific to BU's geo cluster, only your example module load calls are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solved the second point, but I am still working on how to add details into PEcAn documentation.
cdosetup <- "" | ||
if (!is.null(settings$host$cdosetup)) { | ||
cdosetup <- paste(cdosetup, sep = "\n", paste(settings$host$cdosetup, collapse = "\n")) | ||
jobsh <- gsub("@CDO_SETUP@", cdosetup, jobsh) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This statement needs to be outside of the if, otherwise this code will leave the @CDOSETUP@
tag in the job.sh, which will cause the script to fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solved
|
||
# host specific setup | ||
@HOST_SETUP@ | ||
@CDO_SETUP@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dongchen, I see two things that need to happen before I can approve this PR:
- The new tags you've added need to be added to the overall PEcAn documentation, including examples like the one in the thread above so that folks know how to use these new tag
- These tags need to be added to the SIPNET job template, not the generic model template, for them to work. Furthermore, they need to be there in the default job template, not a geo specific one, otherwise your code will fail. Indeed, the problem you've identified and fixed is in no way specific to BU's geo cluster, only your example module load calls are.
Merge branch 'develop' of https://github.com/DongchenZ/pecan into SDA_dongchen # Conflicts: # CHANGELOG.md
…mble members from NEON sites.
Merge branch 'develop' of https://github.com/DongchenZ/pecan into SDA_dongchen # Conflicts: # CHANGELOG.md
Adding template job files specifically for SCC server in BU, and solved several issues so we can not export full year nc files on geo.
Description
Motivation and Context
Review Time Estimate
Types of changes
Checklist: