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

Fix place_candidate.sh #269

Merged
merged 1 commit into from
Mar 19, 2025
Merged

Fix place_candidate.sh #269

merged 1 commit into from
Mar 19, 2025

Conversation

Esteban82
Copy link
Member

I think that ${dataset} was missing on line 69. This line is used to set permissions (chmod). I am not sure if the proposed modification is correct. But I think it makes sense seeing the message of the previous line.

BTW, when I ran the script as is, it failed me with the earth_vgg and earth_synbath grids (and I don't know if any others). I think at the time Paul didn't change the permissions on those datasets. Maybe in the future we will have a problem when it is necessary to update them.

The modified script worked when I tried to run make place-earth-wdmam (#245). The original script failed me for the reasons mentioned in the previous paragraph.

@Esteban82 Esteban82 requested review from joa-quim, seisman and a team March 19, 2025 11:18
Copy link
Member

@joa-quim joa-quim left a comment

Choose a reason for hiding this comment

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

It looks right, though I did not actually tested it.

@Esteban82 Esteban82 merged commit 48f85cd into master Mar 19, 2025
1 check passed
@Esteban82 Esteban82 deleted the Fix-Place_candidate.sh branch March 19, 2025 12:04
@Esteban82
Copy link
Member Author

Yes, it worked for me.

My main doubt is what would happen in the case of doing something like make place-earth. This issue is recorded in case something like this happens.

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