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 default bps configuration for S3DF #26

Merged
merged 1 commit into from Sep 30, 2022
Merged

Conversation

zhaoyuyoung
Copy link
Contributor

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Base: 32.29% // Head: 32.29% // No change to project coverage 👍

Coverage data is based on head (a36665d) compared to base (0ce3e5b).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #26   +/-   ##
=======================================
  Coverage   32.29%   32.29%           
=======================================
  Files           9        9           
  Lines         545      545           
  Branches       93       86    -7     
=======================================
  Hits          176      176           
  Misses        366      366           
  Partials        3        3           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@MichelleGower MichelleGower left a comment

Choose a reason for hiding this comment

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

Some questions and comments. Merge approved.

@@ -35,8 +35,8 @@ payload:

# Specify memory request for executionButler, pipetaskInit and forcedPhotCoadd, placeholder for now
executionButler:
requestMemory: 7000
# queue: "SLAC_Rubin_Merge"
# requestMemory: 7000
Copy link
Contributor

Choose a reason for hiding this comment

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

Just verifying whether the merge queue uses the requestMemory. Because if it does, without this line, the job will have the default which is only 2000.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked on this. The panda queue on S3DF can use requestMemory with the default value 7000. For SDF, user need to follow test_idf.yaml that I added. There requestMemory is set to be 4000, consistent with the configuration of SLAC_Rubin_SDF. All these info will be documented in panda.lsst.io. As Wen suggested, I'll uncomment this line.

# If using SDF: source setup_panda.sh w_2022_32 sdf

latest=$(ls -td /cvmfs/sw.lsst.eu/linux-x86_64/panda_env/v* | head -1)
setupScript=${latest}/setup_panda_s3df.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

This could go in the else of the if clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll move it.

usdf_cluster=$2
if [ "$usdf_cluster" == "sdf" ]; then
setupScript=${latest}/setup_panda.sh
echo "Working on cluster: " $usdf_cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we only want this statement printed if on the sdf cluster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since SDF is only an interim cluster, by default USDF uses S3DF. I think no need to print out for S3DF. Also when source this script, line 13 will show it's using s3df.

@zhaoyuyoung zhaoyuyoung removed the request for review from wguanicedew September 30, 2022 18:06
@zhaoyuyoung zhaoyuyoung merged commit e9ae51c into main Sep 30, 2022
@zhaoyuyoung zhaoyuyoung deleted the tickets/DM-36413 branch September 30, 2022 18:56
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.

None yet

2 participants