-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add partial derivatives and tests #209
Conversation
WalkthroughThe recent updates enhance the derivative module by introducing a new Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- deriv/deriv.v (1 hunks)
- deriv/deriv_test.v (1 hunks)
Additional comments not posted (5)
deriv/deriv_test.v (4)
72-74
: LGTM!The function correctly implements ( f(x, y) = x^2 + y^2 ).
76-78
: LGTM!The function correctly implements ( \frac{\partial f}{\partial x} = 2x ).
80-82
: LGTM!The function correctly implements ( \frac{\partial f}{\partial y} = 2y ).
117-127
: LGTM!The test cases for the partial derivatives of
f_multi
are well-constructed and ensure correctness within a specified tolerance.deriv/deriv.v (1)
117-145
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
partial
match the new signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (4)
deriv/README.md (4)
1-1
: Provide more context for the limitation in performing advanced data analysis.The introductory note is clear, but it would be helpful to explain why advanced data analysis is not possible at the moment.
80-80
: Consider using a shorter alternative for "with respect to".The phrase "with respect to" might be wordy. Consider using "regarding" or "for".
- This function computes the partial derivative of a multivariable function `f` with respect to a specified variable at the point `x` using the central difference method with a step-size of `h`. + This function computes the partial derivative of a multivariable function `f` for a specified variable at the point `x` using the central difference method with a step-size of `h`.Tools
LanguageTool
[style] ~80-~80: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ivative of a multivariable functionf
with respect to a specified variable at the pointx
u...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
82-82
: Consider using a shorter alternative for "with respect to".The phrase "with respect to" might be wordy. Consider using "regarding" or "for".
- The `variable` parameter specifies the index of the variable with respect to which the partial derivative is to be computed. + The `variable` parameter specifies the index of the variable for which the partial derivative is to be computed.Tools
LanguageTool
[style] ~82-~82: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ter specifies the index of the variable with respect to which the partial derivative is to be c...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
84-84
: Avoid redundancy in mentioning the initial value ofh
.The initial value of
h
is mentioned twice, which could be redundant. Consider combining the information into a single sentence.- The initial value of `h` is used to estimate an optimal step-size, based on the scaling of the truncation error and round-off error in the derivative calculation. The partial derivative is computed by converting the multivariable function into a univariate function for the specified variable and then applying the central difference method. + The initial value of `h` is used to estimate an optimal step-size, based on the scaling of the truncation error and round-off error in the derivative calculation. The partial derivative is computed by converting the multivariable function into a univariate function for the specified variable and applying the central difference method.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- deriv/README.md (2 hunks)
Additional context used
LanguageTool
deriv/README.md
[style] ~80-~80: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ivative of a multivariable functionf
with respect to a specified variable at the pointx
u...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
[style] ~82-~82: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ter specifies the index of the variable with respect to which the partial derivative is to be c...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
[style] ~89-~89: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...GSL. 📖 Feel free to explore and utilize these numerical dif...(FEEL_FREE_TO_STYLE_ME)
Markdownlint
deriv/README.md
92-92: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
92-92: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (1)
deriv/README.md (1)
94-96
: LGTM!The instructions for saving the file and the note in Turkish are clear and helpful.
deriv/README.md
Outdated
## References and Further Reading | ||
|
||
This work is a spiritual descendent of the Differentiation module in [GSL](https://github.com/ampl/gsl). 📖 | ||
|
||
Feel free to explore and utilize these numerical differentiation functions in | ||
your projects! 🤖📊🔬 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surround fenced code blocks with blank lines and specify the language.
Fenced code blocks should be surrounded by blank lines and have a language specified to comply with Markdownlint rules.
- ```markdown
+
+ ```markdown
# 🚀 Numerical Differentiation
...
+
Tools
Markdownlint
92-92: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
92-92: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
deriv/README.md (1)
72-74
: Consider simplifying the phrase "with respect to".The phrase "with respect to" is used twice and could be simplified for readability.
- This function computes the partial derivative of a multivariable function `f` with respect to a specified variable at the point `x` using the central difference method with a step-size of `h`. + This function computes the partial derivative of a multivariable function `f` for a specified variable at the point `x` using the central difference method with a step-size of `h`. - The `variable` parameter specifies the index of the variable with respect to which the partial derivative is to be computed. + The `variable` parameter specifies the index of the variable for which the partial derivative is to be computed.Tools
LanguageTool
[style] ~72-~72: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ivative of a multivariable functionf
with respect to a specified variable at the pointx
u...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
[style] ~74-~74: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ter specifies the index of the variable with respect to which the partial derivative is to be c...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- deriv/README.md (1 hunks)
- deriv/deriv_test.v (2 hunks)
Additional context used
LanguageTool
deriv/README.md
[style] ~72-~72: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ivative of a multivariable functionf
with respect to a specified variable at the pointx
u...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
[style] ~74-~74: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ter specifies the index of the variable with respect to which the partial derivative is to be c...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
Additional comments not posted (5)
deriv/deriv_test.v (4)
72-74
: LGTM!The function
f_multi
correctly computes ( f(x, y) = x^2 + y^2 ).
76-78
: LGTM!The function
df_multi_dx
correctly computes ( \frac{\partial f}{\partial x} = 2x ).
80-82
: LGTM!The function
df_multi_dy
correctly computes ( \frac{\partial f}{\partial y} = 2y ).
117-127
: LGTM!The test cases for the partial derivatives of
f_multi
are correctly implemented and use appropriate assertions.deriv/README.md (1)
76-76
: LGTM!The remaining documentation for the
partial
function is clear and well-written.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
deriv/README.md (3)
66-66
: Surround fenced code blocks with blank lines.Fenced code blocks should be surrounded by blank lines to comply with Markdownlint rules.
+ ### `partial` +
68-68
: Specify the language for fenced code blocks.Fenced code blocks should have a language specified to comply with Markdownlint rules.
- ```v ignore + ```v
72-72
: Consider a shorter alternative to 'with respect to'.The phrase 'with respect to' might be wordy. Consider a shorter alternative for clarity.
- This function computes the partial derivative of the function `f` with respect to a specified variable at point `x` using step-size `h`. + This function computes the partial derivative of the function `f` for a specified variable at point `x` using step-size `h`.Tools
LanguageTool
[style] ~72-~72: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... partial derivative of the functionf
with respect to a specified variable at pointx
using...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- deriv/README.md (1 hunks)
Additional context used
LanguageTool
deriv/README.md
[style] ~72-~72: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... partial derivative of the functionf
with respect to a specified variable at pointx
using...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
Additional comments not posted (1)
deriv/README.md (1)
69-69
: Ensure function signature is correct.The function signature appears correct. Ensure it matches the implementation in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some changes, the rest looks great!
deriv/deriv.v
Outdated
*/ | ||
|
||
if variable < 0 || variable >= x.len { | ||
panic('Invalid variable index') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use this function instead of panic https://vlang.github.io/vsl/errors.html#vsl_panic
Co-authored-by: Ulises Jeremias <ulisescf.24@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
deriv/README.md (1)
72-72
: Consider a shorter alternative for "with respect to".The phrase "with respect to" might be wordy. Consider using a shorter alternative like "regarding" or "for".
- This function computes the partial derivative of the function `f` with respect to + This function computes the partial derivative of the function `f` forTools
LanguageTool
[style] ~72-~72: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... partial derivative of the functionf
with respect to a specified variable at pointx
using...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- deriv/README.md (1 hunks)
- deriv/deriv.v (1 hunks)
Additional context used
LanguageTool
deriv/README.md
[style] ~72-~72: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... partial derivative of the functionf
with respect to a specified variable at pointx
using...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
Additional comments not posted (3)
deriv/deriv.v (3)
117-127
: Include the docstring above the function header.To improve readability, include the docstring above the function header.
- pub fn partial(f fn ([]f64) f64, x []f64, variable int, h f64) (f64, f64) { - /** + /** + * partial is a function that computes the partial derivative of a multivariable function with respect to a specified variable. + * + * @param f The multivariable function for which the partial derivative is to be computed. + * @param x The point at which the partial derivative is to be computed, represented as an array of coordinates. + * @param variable The index of the variable with respect to which the partial derivative is to be computed. + * @param h The step size to be used in the central difference method. + * + * @return A tuple containing the value of the partial derivative and the estimated error. + */ + pub fn partial(f fn ([]f64) f64, x []f64, variable int, h f64) (f64, f64) {
130-130
: Usevsl_panic
instead ofpanic
.Use
vsl_panic
as suggested in the previous review comment.- panic('Invalid variable index') + vsl_panic('Invalid variable index')
117-145
: LGTM!The implementation of the
partial
function is correct and follows the intended logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
everything is done! thank you for your time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
deriv/deriv_test.v (1)
Line range hint
1-1
: Reminder: Format the file.Run
v fmt -w deriv/deriv_test.v
to format the file.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- deriv/deriv.v (2 hunks)
- deriv/deriv_test.v (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- deriv/deriv.v
Additional comments not posted (4)
deriv/deriv_test.v (4)
71-73
: LGTM!The function
f_multi
correctly calculates ( f(x, y) = x^2 + y^2 ).
75-77
: LGTM!The function
df_multi_dx
correctly calculates ( \frac{\partial f}{\partial x} = 2x ).
79-81
: LGTM!The function
df_multi_dy
correctly calculates ( \frac{\partial f}{\partial y} = 2y ).
116-126
: LGTM!The test cases correctly validate the
partial
function by comparing its output with the expected partial derivatives off_multi
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* 'main' of github.com:vlang/vsl: Replace panic with vsl_panic in graph.v (#214) Replace panic with vsl_panic in eval function (#212) change IImage.data from voidptr to &u8 Add Planck Temperature to Constants (#210) Add partial derivatives and tests (#209) ci: comment out the whole super-linter job (too many false positives, it seems that the tool is not configured properly) ci: update Dockerfile to satisfy the lint job ci: change `master` to `main` in .github/workflows/lint.yml ci: upgrade to `super-linter/super-linter/slim@v6.7.0` fix `v check-md ~/.vmodules/vsl` fix compilation on macos with latest clang 15 and LAPACK from brew
deriv: add partial derivative function and tests
This PR adds the
partial
function to compute partial derivatives for multivariable functions. It also includes corresponding test cases to verify the correctness of the new function.The new
partial
function calculates the partial derivative of a given multivariable function with respect to a specified variable using the central difference method. This addition enhances the library's ability to handle multivariable calculus operations effectively.The following test cases are added to ensure the correctness of the new function:
Summary by CodeRabbit
New Features
Tests