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

JOSS paper #76

Merged
merged 175 commits into from
Oct 5, 2023
Merged

JOSS paper #76

merged 175 commits into from
Oct 5, 2023

Conversation

jagoosw
Copy link
Collaborator

@jagoosw jagoosw commented Mar 15, 2023

To do:

  • Finish writing paper
  • Fix API documentation (some not being automatically imported for some reason)
  • Put package on Pkg
  • Make the OVS poster figures more size/shape appropriate
  • [ ] Check I've not broken box models - examples double as tests
  • Fix figure

@jagoosw
Copy link
Collaborator Author

jagoosw commented Mar 17, 2023

Docs now fixed and more complete #77

@jagoosw
Copy link
Collaborator Author

jagoosw commented Mar 18, 2023

Now I've compiled the draft I do think I need to make the figure font sizes larger

@johnryantaylor
Copy link
Collaborator

johnryantaylor commented Mar 18, 2023 via email

@jagoosw
Copy link
Collaborator Author

jagoosw commented Sep 24, 2023

I think this is now finalised if everyone is happy? @johnryantaylor

@navidcy
Copy link
Collaborator

navidcy commented Sep 24, 2023

I'm happy. Could you perhaps resolve the conflicts in simple_multi_G.jl?

@jagoosw
Copy link
Collaborator Author

jagoosw commented Sep 24, 2023

I'm happy. Could you perhaps resolve the conflicts in simple_multi_G.jl?

done

@jagoosw jagoosw marked this pull request as ready for review September 24, 2023 18:17
@jagoosw
Copy link
Collaborator Author

jagoosw commented Sep 24, 2023

I might have deleted a change you made to the sediment thing actually @navidcy ?

@jagoosw
Copy link
Collaborator Author

jagoosw commented Sep 24, 2023

Once we're done we need to delete the draft workflow before merging too

Comment on lines 192 to 193
sediment.denitrification_params.E * log(k) ^ 2 +
sediment.denitrification_params.F * log(O₂) * log(k)) / (Cᵐⁱⁿ * day)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought not having k as variable here was crucial, no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh now I see this is commented out code.
do we need commented code?

Comment on lines 198 to 199
sediment.anoxic_params.D * log(k) +
sediment.anoxic_params.E * log(O₂) * log(k) +
Copy link
Collaborator

Choose a reason for hiding this comment

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

k as variable?

Copy link
Collaborator Author

@jagoosw jagoosw Sep 25, 2023

Choose a reason for hiding this comment

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

This is fixed in #138 not here (would be good to merge 138 soon)

@jagoosw jagoosw merged commit 83e11aa into main Oct 5, 2023
4 checks passed
@jagoosw jagoosw deleted the joss-paper branch October 5, 2023 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants