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 vignette and analysis #259

Merged
merged 22 commits into from
Nov 26, 2024
Merged

Update vignette and analysis #259

merged 22 commits into from
Nov 26, 2024

Conversation

marcadella
Copy link
Collaborator

No description provided.

fabern and others added 7 commits November 22, 2024 01:16
Because of computational reasons some cells of the vignettes are not rerun each time. They rely on intermediately stored files. While this is error-prone, it had been chosen as a computationally pragmatic way.

Here, we update the results and streamline the generation of the result files. The aim is to make more evident where each result was generated and is used.

Lastly the sensitivity vignette has been changed to calibration of kphio, kphio_par_b, and kc_jmax; instead of kphio, kc_jmax, and soilm_betao.
- Remove sitename from biomee drivers' site_info as it was clashing with the top level key (when unnesting)
- Use parallel execution to calibrate models (not necessary faster but at least it prints out the progress)
- Update analysis to calibrate new parameter set for morris analysis.
- Bump version to 5.0.0
R/calib_sofun.R Outdated Show resolved Hide resolved
@fabern
Copy link
Member

fabern commented Nov 26, 2024

@stineb, @marcadella
What version number will this be? Have you already discussed this?
5.0.0 or 4.5.0?

According to semver, breaking API changes would require a 5.0.0. I believe we had breaking changes for BiomeE input drivers, right?

Note that currently both 5.0.0 and 4.5.0 appear: DESCRIPTION vs NEWS.md.

@stineb
Copy link
Collaborator

stineb commented Nov 26, 2024

I've discussed this with @marcadella . Yes, it should be 5.0.0.

R/calib_sofun.R Outdated Show resolved Hide resolved
cran-comments.md Show resolved Hide resolved
@marcadella marcadella marked this pull request as ready for review November 26, 2024 15:16
@marcadella marcadella requested a review from fabern November 26, 2024 15:16
@marcadella
Copy link
Collaborator Author

Alright, I think we are finally ready. Once this PR is merged in Master, I'll submit to CRAN.

Copy link
Member

@fabern fabern left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks @marcadella

fabern
fabern previously approved these changes Nov 26, 2024
Copy link
Member

@fabern fabern left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks @marcadella

Copy link
Member

@fabern fabern left a comment

Choose a reason for hiding this comment

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

All ready for merging once the CI is through.

@marcadella marcadella merged commit 3a1a001 into master Nov 26, 2024
7 checks passed
@fabern fabern deleted the update-vignette-files branch November 26, 2024 17:54
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.

3 participants