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

135 cablenml improvements #136

Merged
merged 13 commits into from
Sep 15, 2023
Merged

135 cablenml improvements #136

merged 13 commits into from
Sep 15, 2023

Conversation

tammasloughran
Copy link
Collaborator

@tammasloughran tammasloughran commented Jun 27, 2023

CABLE

Thank you for submitting a pull request to the CABLE Project.

Description

The cable.nml documentation can be improved.

  • Many entries in the description table do not actually have default values.
  • Some default values are wrong. Eg. vegparmnew = .FALSE.
  • Some namelist variables are missing.
  • Why is the default restart_in = ' ' ? What does it mean?
  • The table widths could be adjusted.
  • It looks like filename%veg and filename%soil are not used.

Fixes #135

Type of change

Please delete options that are not relevant.

  • New or updated documentation

Checklist

  • The new content is accessible and located in the appropriate section.
  • I have checked that links are valid and point to the intended content.
  • I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


📚 Documentation preview 📚: https://cable--136.org.readthedocs.build/en/136/

@tammasloughran tammasloughran linked an issue Jun 27, 2023 that may be closed by this pull request
@tammasloughran
Copy link
Collaborator Author

I think I'm at the point where I need others to fill in the missing descriptions.
Should the namelist variables related to POP stay here? or is there a separate location for POP related documentation?

@JhanSrbinovsky
Copy link
Collaborator

I suppose cablenml.md should reflect cable.nml, which is in desperate need of revision. At least in the MD we can organise things into sections which will eventually be namelists in their own right. For e.g. Anything to do with filename% and output% doesn't belong in a generic cable.nml. The UM doesn't use any of it. We should have casa.nml and pop.nml (etc etc) files. You're right to get rid of the filename%veg/soil.

@ccarouge
Copy link
Collaborator

ccarouge commented Jul 6, 2023

I agree we should put some order into this but this PR was to look at the correctness of the information. I think there is already enough done here and don't need to add. I will put your comment @JhanSrbinovsky as an issue so we can discuss there how to organise the documentation for the namelist.

Copy link
Collaborator

@ccarouge ccarouge left a comment

Choose a reason for hiding this comment

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

A few changes, especially some duplicated entries. But otherwise it's good work.

documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
@tammasloughran
Copy link
Collaborator Author

I'm having difficulty understanding what cable_user%soil_thermal_fix is. Does anyone know what this is?

@JhanSrbinovsky
Copy link
Collaborator

JhanSrbinovsky commented Jul 7, 2023

I'm having difficulty understanding what cable_user%soil_thermal_fix is. Does anyone know what this is?

From memory an alternative soil conductivity implementation from Mark Deker - the Ground Water guy. I did think we were using it in CM2, but it doesn't appear so.

@tammasloughran
Copy link
Collaborator Author

I don't know what cable_user%initialize_mapping is either. @JhanSrbinovsky , it looks like you wrote this?

@tammasloughran
Copy link
Collaborator Author

cable_user%cable_runtime_coupled seems to only do some snow stuff when uncoupled. Why and What?

@JhanSrbinovsky
Copy link
Collaborator

JhanSrbinovsky commented Jul 14, 2023

I don't know what cable_user%initialize_mapping is either. @JhanSrbinovsky , it looks like you wrote this?

You can kill this. It is a relic from saving the ACCESS forcing. It has been useful(potentially will be again) as a diagnostic tool. But in no way needed for the model.

@JhanSrbinovsky
Copy link
Collaborator

cable_user%cable_runtime_coupled seems to only do some snow stuff when uncoupled. Why and What?

This one is still in the code, largely redundant yet requires further thought. We flipped flopped over the default for a while and it's one of those things that I have to start all over again to understand and remove it. It is a relic from CMIP5 which initially started from a dead cold state, i.e. tiled soils etc didn't exist in the UM, had no values, let alone spun up. Then it morphed as CMIP5 UM didn't keep track the date and we needed to dodge it (we had to have a post-processing date tracker - it was crazy). Then it resurfaced in CMIP6 with a sightly different usage requirement again. Gives me a headache just thinking about it. Kill it and maybe @ccarouge can flag it?

@tammasloughran
Copy link
Collaborator Author

tammasloughran commented Jul 14, 2023

Many thanks! The only other entries I can't make sense of are the GW related flags.

  • cable_user%litter
  • cable_user%gw_model
  • cable_user%or_evap
  • cable_user%test_new_gw

Can someone that knows these well can describe them for me?

@ccarouge
Copy link
Collaborator

@JhanSrbinovsky I'm not certain what you mean by "kill it" and "flag it". I would assume "kill it" means remove it? But "flag it", I have no idea.

@JhanSrbinovsky
Copy link
Collaborator

JhanSrbinovsky commented Jul 14, 2023 via email

@ccarouge
Copy link
Collaborator

OK. So I've added an issue in this repository (#137 ) to add the deprecated namelist options to the list Ramzi has started in the User Guide. We can add to that issue as we find other such options.

@ccarouge
Copy link
Collaborator

  • cable_user%litter

All I know is that it adds a litter layer on top of the soil surface. It was implemented because CABLE was/is over-evaporating and this reduces the evaporation. But that might not be useful information for the User Guide.

  • cable_user%gw_model

This is to turn on an alternate soil model that takes into account the groundwater. @bibivking How best would you describe this in the documentation when describing the namelist options?

  • cable_user%or_evap

This is an evaporation scheme from Dani Or. But I don't know if there is a reference for it. @bibivking Do you know more about this scheme?

  • cable_user%test_new_gw

I would think this one will disappear when Ramzi is finished merging the GW scheme. The technical phase should be finished by the end of August, so I would put this in deprecated and not list it here. @rkutteh any thoughts on this namelist option? Have you encountered it so far in your work merging the versions?

@rkutteh
Copy link
Collaborator

rkutteh commented Jul 29, 2023

cable_user%litter
cable_user%gw_model
cable_user%or_evap
cable_user%test_new_gw

Of the four above, the only one I have not encountered yet is the last one (but I am midway through the work still...). The second and third are definitely in use and needed. I will get back to this thread in a month or so when I am finished with moving the GW into the trunk and say more...

@tammasloughran
Copy link
Collaborator Author

tammasloughran commented Aug 16, 2023

I have updated everything with the information provided, thanks folks! It looks like all the namelist variables here have some sort of description.
Should there be an order? Alphabetical? Data type? The most important ones up the top? Or is it fine as it is?
Can the table be sortable by category in the webpage rendered by mkdocs markdown?

@ccarouge
Copy link
Collaborator

@tammasloughran I don't think we want to worry about an order now. We will want to split this huge namelist into several namelists soon. The ordering would happen then. The webpage rendered is static so no ordering possible there.

@ccarouge
Copy link
Collaborator

@tammasloughran Is it ready for another review? If so, can you request it? Either look at the "Reviewers" menu on the top right and click the "2 arrows in a circle" icon, a bit like this: 🔄
Or at the bottom of the page, where it says "1 change requested", expand that portion, click on the "3 dots icon" and choose "Re-request review".

Copy link
Collaborator

@ccarouge ccarouge left a comment

Choose a reason for hiding this comment

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

I have minor suggestions to make things a bit clearer.

I would prefer if the table wasn't between the example for offline and ACCESS simulations. So if you can move either the table first and the examples after or the other way around that would be great.

But I have approved as is so feel free to merge in when you think it is ready.

documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
documentation/docs/user_guide/inputs/cable_nml.md Outdated Show resolved Hide resolved
@tammasloughran
Copy link
Collaborator Author

I'm not sure what to do about the link checks. At first I thought there was some problem with the DOI redirection but changing it to a full URI didn't make any difference.

@ccarouge ccarouge merged commit 9563063 into main Sep 15, 2023
2 checks passed
@ccarouge ccarouge deleted the 135-cablenml-improvements branch September 15, 2023 09:07
@ccarouge
Copy link
Collaborator

It's possible to ignore specific links in the link checker: https://github.com/gaurav-nelson/github-action-markdown-link-check#disable-check-for-some-links . So I've added this for the link to the paper. I've changed the link to go via doi.org so it will stay valid even if the journal is sold.

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.

cable.nml improvements
4 participants