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

Fix23 #27

Merged
merged 6 commits into from
Apr 5, 2024
Merged

Fix23 #27

merged 6 commits into from
Apr 5, 2024

Conversation

jialinl6
Copy link
Contributor

@jialinl6 jialinl6 commented Mar 29, 2024

Fixes #23
captures #23

Copy link

codecov bot commented Mar 29, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 80.36%. Comparing base (a0d5144) to head (5aa4c11).
Report is 1 commits behind head on main.

Files Patch % Lines
src/VBS.jl 60.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
- Coverage   84.17%   80.36%   -3.81%     
==========================================
  Files          11       11              
  Lines         139      163      +24     
==========================================
+ Hits          117      131      +14     
- Misses         22       32      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@ctessum ctessum left a comment

Choose a reason for hiding this comment

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

Thanks! Just a couple of suggestions above. Also it would be good if you can add a documentation page that describes the algorithm and demonstrates how to use it, for example including the analysis in Kai's spreadsheet that reproduces the figure in the original paper.

src/VBS.jl Show resolved Hide resolved
src/VBS.jl Show resolved Hide resolved
Copy link
Member

@ctessum ctessum left a comment

Choose a reason for hiding this comment

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

Thanks! Just a couple of suggestions above. Also it would be good if you can add a documentation page that describes the algorithm and demonstrates how to use it, for example including the analysis in Kai's spreadsheet that reproduces the figure in the original paper.

@ctessum
Copy link
Member

ctessum commented Apr 1, 2024

Also you can delete the file src/Low VOC, because this should supersede that VBS version.

@ctessum
Copy link
Member

ctessum commented Apr 1, 2024

This project is governed with Gov4Git. Gov4Git notices

On Monday, 01-Apr-24 15:11:10 UTC by Gov4Git dev

Notice fvs7ij

Started managing this PR as Gov4Git proposal 27 with initial approval score of 0.000000.

This project is managed by Gov4Git, a decentralized governance system for collaborative git projects.
To participate in governance, install the Gov4Git desktop app.

@jialinl6
Copy link
Contributor Author

jialinl6 commented Apr 4, 2024

The documentation looks like this: VBS · Aerosol.jl.pdf

Copy link
Member

@ctessum ctessum left a comment

Choose a reason for hiding this comment

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

Thanks!

@ctessum
Copy link
Member

ctessum commented Apr 5, 2024

This project is governed with Gov4Git. Gov4Git notices

On Friday, 05-Apr-24 20:24:44 UTC by Gov4Git dev

Notice pdayz2

This PR's approval score is now 3.000000.
The cost of review is 9.000000.
The projected bounty is now 0.000000.

Notice u5i4ul

The set of eligible issues claimed by this PR is now empty.

@ctessum ctessum merged commit 8792322 into EarthSciML:main Apr 5, 2024
2 of 7 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 5, 2024
@ctessum
Copy link
Member

ctessum commented Apr 5, 2024

This project is governed with Gov4Git. Gov4Git notices

On Friday, 05-Apr-24 20:38:34 UTC by Gov4Git dev

Notice dxkvjc

This PR, managed as Gov4Git proposal 27, has been closed 🎉

The PR approval tally was 3.000000.

Bounty of 0.000000 credits was donated to the community's matching fund.

Resolved issues:

Rewarded PR reviewers:

  • Reviewer @ctessum was awarded 9.000000 credits

Tally breakdown by user:

  • User @ctessum contributed 3.000000 votes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add VBS SOA algorithm
2 participants