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

Create kriged apportioned abundance documentation #216

Conversation

brandynlucca
Copy link
Collaborator

This includes a draft of the apportion_abundance document that covers how kriged biomass is reallocated into estimates of abundance, which is not to be confused with apportioning abundance estimates that were, themselves, kriged. A variety of images have been included that can be swapped out for equations.

@leewujung
Copy link
Member

There's some weird missing ")" in the functions that is probably not used -- see https://github.com/OSOceanAcoustics/echopop/actions/runs/8455613434/job/23163503465?pr=216

I think it'll be good to resolve that before we merge this PR so that the main does not get broken.

@leewujung
Copy link
Member

Ok, so I think you branched out after you did the code changes, so the 2 PRs are mingled. I fixed it by removing the 2 testing_* files and fixed the missing ")" just so that this PR is clean.

@leewujung leewujung self-assigned this Apr 14, 2024
@leewujung leewujung marked this pull request as ready for review September 27, 2024 02:27
@leewujung
Copy link
Member

See #218 for indexing!

Copy link
Member

@leewujung leewujung left a comment

Choose a reason for hiding this comment

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

I did a PR to fix the indexing problems and simplified the text in brandynlucca#6.

I also created an issue #279 to reconcile repeated materials and notation differences between the 2 apportioning pages -- I'll take that on once this one is merged.

leewujung and others added 4 commits October 16, 2024 13:42
* fix indexing and simplify description

* hyperlink and text tweak

* small fixes

* fix heading levels, fix indexing in first section

* remove unused images

* fix equation rendering

* fix attention

* change sequence of text and note

* revert eq.4-7 to write out male and female expressions

* tweak back calculation section

* change page name

* Apply suggestions from code review

Co-authored-by: Brandyn Lucca <[email protected]>

---------

Co-authored-by: Brandyn Lucca <[email protected]>
@leewujung
Copy link
Member

Hey @brandynlucca - Why is sphinx.ext.autodoc from this? Seems out of the blue and results into a conflict?

Copy link
Member

@leewujung leewujung left a comment

Choose a reason for hiding this comment

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

I think we can merge this now. I will submit a PR to add stratum $i$ to the Jolly-Hampton section, add the citation properly, make some symbol and terminology changes, and not the mapping of each variables to the original equations.

@brandynlucca
Copy link
Collaborator Author

Hey @brandynlucca - Why is sphinx.ext.autodoc from this? Seems out of the blue and results into a conflict?

To clarify: are you asking why it was added to _config.yml? If so, it was previously commented out in this PR and when trying to build the jupyter-book I ran into an error that informed me that the autodoc extension was missing. I think it may have been previously commented out to try and debug possible extension issues; however, I'm not sure why it is reporting a conflict here.

@leewujung
Copy link
Member

leewujung commented Oct 18, 2024

Apparently I was missing a word in my comment, but I also misread the code, there was a "#" in front of the line, so there was no effect.

Could you pull in main and resolve the conflicts?

Also, on a second thought, could you remove the Jolly-Hampton stuff from this PR and submit it as a separate PR? I am likely not able to work on it until after next Wednesday because of a short-notice presentation that day, but I think we should release without that part of docs.

@brandynlucca
Copy link
Collaborator Author

Also, on a second thought, could you remove the Jolly-Hampton stuff from this PR and submit it as a separate PR? I am likely not able to work on it until after next Wednesday because of a short-notice presentation that day, but I think we should release without that part of docs.

Done! This corresponds to #281 now.

@brandynlucca brandynlucca merged commit 39c8817 into OSOceanAcoustics:main Oct 18, 2024
6 checks passed
@leewujung
Copy link
Member

Remember to use "squash and merge" next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Questions concerning back-calculated abundance apportionment
2 participants