Added Jacobian Features in Calculus functions.py

[Kritika75]

Enhanced the jacobian function to include additional features: computation of Hessian matrices for single functions, optional sparse matrix representation for large systems, and the ability to calculate the rank of the Jacobian matrix.

Fixes #39218

📝 Checklist

• The title is concise and informative.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation and checked the documentation preview.

⌛ Dependencies

[vincentmacri]

@tscrim (since I know you have the permission to approve workflows from first-time contributors) would you be willing to take a look at this? Or approve the CI workflows so I can review it?

[github-actions]

Documentation preview for this PR (built with commit 59959e6; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[tscrim]

@vincentmacri It has been done by someone else. Please go ahead and do the review. I can re-approve workflows as needed.

[vincentmacri]

I think you meant:

Suggested change

	jacobian_matrix([[diff(f, v) for v in variables] for f in functions])
	jacobian_matrix = matrix([[diff(f, v) for v in variables] for f in functions])

[vincentmacri]

But with the other changes I suggested you should just revert the jacobian function back to how it was, and add a hessian function for that functionality.

[vincentmacri]

I think that hessian should be its own function unless you have a good reason for it not to be separate.

I don't think we need the rank option since the returns matrix already has a .rank() method that can be called on it.

[vincentmacri]

I also don't see why the sparse option is needed when you could just call .sparse_matrix() on the returned matrix.

[vincentmacri]

@Kritika75 In summary, unless there is some good reason I'm missing for why you implemented things the way you did, can you make these three changes?

1. Remove the hessian option and add a hessian function instead.
2. Remove the rank option, it's unnecessary since matrices already have a .rank() method. It also complicates type checking if the function can possibly return two different types (either a matrix or a tuple).
3. Remove the sparse option, it's unnecessary since matrices already have a .sparse_matrix() method.

Since you're a first-time contributor, GitHub is kind of annoying about running the CI tests on your PR and someone needs to approve every test run manually. To speed things up, please test things locally and make sure your tests pass before requesting another review.

[tscrim]

I also don't see why the sparse option is needed when you could just call .sparse_matrix() on the returned matrix.

You don't want to create a dense matrix just to make it sparse. This is wasteful in both memory and computation time.

[tscrim]

That being said, the code itself should directly create thr sparse matrix, otherwise it should not have such an option as @vincentmacri said.

Although in nearly all cases, this will not be a sparse matrix. So there might not be much practicality in having it (and hence, the maintenance burden of such code).

[Kritika75]

Apologies for the delay in getting back to you. I had my exams, but I completely understand your concern and will address it promptly

[vincentmacri]

One small change request. Looks good otherwise!

[vincentmacri]

@Kritika75 also make sure to pull the latest changes from develop, otherwise git won't let us merge this.

[vincentmacri]

@Kritika75 you need to merge the changes from develop into your patch-1 branch.

[vincentmacri]

If you need a quick git tutorial.

1. Make sure the develop branch of your fork is up to date with the sage develop branch
2. On your machine, from the develop branch run git pull. Then change to your patch-1 branch and run git merge develop. Then git push.

[vincentmacri]

Looks good, all that's left is to run the CI and make sure the tests pass. @tscrim can you approve the workflow run?

[vincentmacri]

@Kritika75 looks like you forgot to import your new hessian function in calculus/all.py. Import the hessian function where the jacobian function is imported in that file. To save us some time, please compile sage and test it locally as well. When you've verified that it works locally tag @tscrim and ask him to approve your workflow run.

[vincentmacri]

I should have checked this earlier, but I just noticed that we already have a hessian function in Sage (https://doc.sagemath.org/html/en/reference/calculus/sage/symbolic/expression.html#sage.symbolic.expression.Expression.hessian), but it's a method that can be called on expressions rather than a top-level function. I do find it a bit odd that the Hessian which is so closely related to the Jacobian is accessed in a different way. Perhaps the hessian function being added here should just call the existing hessian function? Or maybe we don't need a new hessian method after all. @tscrim do you have any thoughts?

However the existing hessian method for expressions lacks the ability to specify the order of the variables to differentiate with, which is something that the implementation here does provide. We could add an (optional) argument to the hessian method for expressions to specify the order of the variables. The existing gradient method for expressions (which is the Jacobian on a single function) does allow the user to specify the order of the variables.

[vincentmacri]

I definitely want the new functionality here to compute the Hessian matrix with respect to the variables in a specified order to be added to sage/symbolic/expression.pyx.

I'm not sure if it's worth having a hessian function in calculus.py which is just a wrapper for the expression one. I'm leaning towards no but open to other opinions.

[vincentmacri]

I thought this over a bit, here's where I think we're at:

Your hessian function in calculus.py looks good. However, we already have a hessian function in symbolic/expression.pyx, and that's a better place than calculus.py for the hessian function.

However, the hessian function in symbolic/expression.pyx is missing the ability to pass in the variables to differentiate with respect to, and in what order. This means that the order of the variables is fixed, which realistically isn't a big deal (but would still be a nice feature to have). More importantly, all variables (in the sense of Sage variables) in the expression are treated as variables (in the mathematical sense of being things that vary). So if the user wants to compute the Hessian of a * x^2 with a being considered to be a constant, they can't do that right now. This would be a nice feature to have for the hessian function.

To address all this, I want you to do these three things:

1. Remove your hessian function from calculus.py. The code itself is good, but I don't think calculus.py is the right place for the hessian function. I do think there is something to say for the Hessian being closely related to the Jacobian and making it easy to find from there. To address that, you can add a SEEALSO block to the docstring of the Jacobian function pointing users to the hessian function for the Expression type.
2. In symbolic/expression.pyx, add an optional keyword variables to the hessian function. This keyword should behave in the same way as the variables keyword for the existing gradient function there.
3. Add some examples to the docstring for hessian in expression.pyx that illustrate the two new abilities you've added to the hessian function:
   • a pair of examples showing how you can change the order of the variables that the Hessian is computed with respect to
   • an example showing what happens if you only specify some of the variables that exist in the expression (the function should compute the Hessian matrix as if the other variables are unknown constants)

My apologies for not noticing the existing hessian function earlier.

[Kritika75]

I thought this over a bit, here's where I think we're at:

Your hessian function in calculus.py looks good. However, we already have a hessian function in symbolic/expression.pyx, and that's a better place than calculus.py for the hessian function.

However, the hessian function in symbolic/expression.pyx is missing the ability to pass in the variables to differentiate with respect to, and in what order. This means that the order of the variables is fixed, which realistically isn't a big deal (but would still be a nice feature to have). More importantly, all variables (in the sense of Sage variables) in the expression are treated as variables (in the mathematical sense of being things that vary). So if the user wants to compute the Hessian of a * x^2 with a being considered to be a constant, they can't do that right now. This would be a nice feature to have for the hessian function.

To address all this, I want you to do these three things:

1. Remove your hessian function from calculus.py. The code itself is good, but I don't think calculus.py is the right place for the hessian function. I do think there is something to say for the Hessian being closely related to the Jacobian and making it easy to find from there. To address that, you can add a SEEALSO block to the docstring of the Jacobian function pointing users to the hessian function for the Expression type.

2. In symbolic/expression.pyx, add an optional keyword variables to the hessian function. This keyword should behave in the same way as the variables keyword for the existing gradient function there.

3. Add some examples to the docstring for hessian in expression.pyx that illustrate the two new abilities you've added to the hessian function:

   • a pair of examples showing how you can change the order of the variables that the Hessian is computed with respect to
   • an example showing what happens if you only specify some of the variables that exist in the expression (the function should compute the Hessian matrix as if the other variables are unknown constants)

My apologies for not noticing the existing hessian function earlier.

Ok, I will do it, thanks for the clarification!