Closed DaniJonesOcean closed 3 years ago
Hi Dan
The normal way to run these would be ./testreport -ad -t global_oce_cs32
as for the other verification experiments. Seems to work fine so I am not sure there would be a need to update prepare_run
.
Maybe some confusion came from the fact that not all setups in verification_others/
work with testreport
while all in MITgcm/verification/
do and global_oce_cs32
falls in that category.
So maybe we indeed need to add to this README -- the ./testreport -t global_oce_cs32
and ./testreport -ad -t global_oce_cs32
+ references to the relevant doc sections maybe?
Not sure about adding detailed directions here just for just this one sub-experiment though (what about the forward and the others in cs32? the llc90 ones? etc). Plus this would add need to maintain consistency between README and prepare_run in future.
ps. it also seems very unfortunate that testreport
is not prominent in sections 3 and 4 of the main manual and there isn't a README.md at the top level of verification_others
pps. I do not have privileges to assign reviewers, merge PRs, etc but hopefully one of the org owners will take charge this PR soon
Hi @DanJonesOcean @gaelforget I think one issue here is that the prepare_run does some of the work here but testreport does some of the other prep steps... and @DanJonesOcean was just trying to spell out ALL the steps necessary w/o invoking testreport (some of which are in prepare_run) in this README? [That being said, looking at prepare_run, it wasn't immediately clear to me how files in input_ad.sens were copied over, maybe testreport grabs any input_ad*/ files?]. @DanJonesOcean is this assessment correct? So I was going to add a mention of using prepare_run at the end of the new paragraph in README.
In terms of Gael's other manual suggestions, for Ch 10 I'll defer to what you think best if you submit a PR for this, but not sure what should be added in doc ch 5 testreport discussion (?) In ch 3 'getting started' I use an example that does not include a prepare_run script, and indeed prepare_run most thoroughly mentioned as part of the testreport doc (which is oriented at a code developer); but I do also mention prepare_run at onset of ch 4. Perhaps there should be better and more consistent running directions in all the verification experiment README files (including a testreport command line as you describe above) albeit this would take some effort.
Hi @jrscott and @gaelforget. Thanks for taking a look at my pull request.
@DanJonesOcean was just trying to spell out ALL the steps necessary w/o invoking testreport (some of which are in prepare_run) in this README
Yes, that's correct. I was using this experiment as a standalone case for some model development, not as a verification experiment. So I wasn't using the "testreport" approach.
Every now and then, I use the "verification" and "verification_other" experiments as starting configurations for model development. That's what I was doing here, and I was thinking that it could be good for other users to avoid some of the omissions/mistakes that I made while building this experiment. @gaelforget, I think I see what you're saying about the "normal" way to run these, but I guess it's not really for me to decide. I'll let others make that call.
I was going to add a mention of using prepare_run at the end of the new paragraph in README.
Sounds good!
In terms of Gael's other manual suggestions ...
Yes, @gaelforget has raised some good points about the documentation. I probably won't be the one to take that one forward right now, to be honest!
Could I ask for more thoughts on this? I personally found these steps useful when trying to build a new experiment using this verification exercise as a starting point. That's why I suggested that they be included.
How would you like to proceed?
Thanks. : )
Hi @DanJonesOcean
I find your addition useful. Maybe you can add a reference to the prepare_run
script (that it basically does the steps you spell out), and that your instructions are explicitly for the case that you don't want use testreport on this. I also like Gael's suggestion of adding the testreport
line to the README (before your new section).
And then add the entire part also to the README.md in the global_oce_llc90
experiment? I don't think that there are any other README.md here.
@DanJonesOcean I agree that a little bit more information in the current "README.md" file would be usefull. Like for some README in MITgcm/verification/ experiments, it could mention the different steps to a) download the set-up (and this requires more actions than for standard MITgcm/verification/ experiments), b) how to compile and run, assuming the user do not use testreport (like for other REAME files). I think it's useful to recommend the use of "prepare_run", because it make things easier + increase the chances that the instructions will still be valid even after the list of files to link change (in the future). And while you may also consider updating global_oce_llc90 README.md file, I would recommend to focus on this one (global_oce_cs32) first until it's ready to merge before starting to update the llc90 one.
FWIW, the READMEs in the verification experiments need an broader overhaul, but I don't think we've had a formal discussion what they should include. @jm-c has good ideas above. One thing I would make a point to add in general is what unique aspects/code a given verification exp uses. (make this a point a new issue?) But often this will be useful info on a case-by-case basis and this is valuable info, thx @DanJonesOcean
Just a comment here (not sure how useful it might be): I use a slightly modified set of instructions compared to Gael's one: I generally download "verification_other" after MITgcm, and put it inside MITgcm/, so that it's at the same level in dir tree as verification/. This is how it's done on reference machine "villon.mit.edu": http://wwwcvs.mitgcm.org/viewvc/MITgcm/MITgcm_contrib/test_scripts/ref_machine/test_villon?view=log with adjusted instructions here: http://wwwcvs.mitgcm.org/viewvc/MITgcm/MITgcm_contrib/test_scripts/ref_machine/settings.txt?view=log And tested every night, e.g. here: http://mitgcm.org/testing/results/2020_12/tr_villon-a_20201223_1/summary.txt
@DanJonesOcean
I cannot make any changes to this PR, because remote: Permission to so-wise/verification_other.git denied to mjlosch.
If that cannot be solved I suggest to close this PR and open another one with proper permissions.
Hi @mjlosch. Thanks for offering to take a look.
Oddly, I don't have an "Allow edits by maintainers" checkbox under the list of participants on this pull request. The documentation I've seen suggests that this box should exist...
One difference: my branch is on an organisational repository as opposed to a personal one. I don't see why this should matter, as I'm an owner of the organisational repository. I'll try a couple things and report back. Happy to take suggestions if anyone knows how to solve this permissions issue.
Okay, I'll take @mjlosch's suggestion and close this PR. Perhaps a PR from my personal account (as opposed to my organisation) will be easier to change in terms of permissions.
Thanks @DanJonesOcean if the new attempt is also problematic we'll brainstorm on our end.
This pull request is an attempt to resolve this issue.
I have updated the README.md file to include some additional run setup procedures. The commands and description above should help users set up and run the generic integral function test case. I have verified that those instructions will allow the user to set up and execute the run, as long as they have already compiled the executable.
This pull request is only related to the documentation. There is also a
prepare_run
script that could be updated, but perhaps that should be a separate issue and pull request.I hope this is okay - this is my first attempt at a pull request! Please let me know if I need to change anything.