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

We now allow src/build_function to take NamedTuples as a datastructure for building a function #929

Merged

Conversation

raphaelchinchilla
Copy link
Contributor

This is a simple addition to build_function that dramatically increases its versatility. Here is a small example:

@variables x[1:2] y[1:2]
op=x+y
z=(x=x,y=y)
code_fun=build_function(op,z)[2]

code_fun is:

:(function (ˍ₋arg1,)
      begin
          (broadcast)(+, ˍ₋arg1.x, ˍ₋arg1.y)
      end
  end)

This is useful because the built function uses the "." to access the elements of the input, for instance _-arg1.x. So we can pass to the built function any datastructure where the elements are accessed using ".", which obviously includes NamedTuples, but also structs and ComponentArrays.

@ChrisRackauckas
Copy link
Member

Can you add a test?

@raphaelchinchilla
Copy link
Contributor Author

Yes. Do they need to be as thorough as the ones in /test/build_function_arrayofarray ? Or should I just include some additional tests?

@raphaelchinchilla
Copy link
Contributor Author

Hi all, I have had some busy days and did not have time yet to include the tests.

Please correct me if I am wrong, but the build error is not related to my PR, right?

…n_arrayofarray.jl but the numeric inputs as well as the symbolic inputs are NamedTuples. I verified that without the change in build_function in this PR, these tests all fail.
@raphaelchinchilla
Copy link
Contributor Author

@ChrisRackauckas I just included the tests that you asked me to.

They are an exact copy of the test for build_function_arrayofarray.jl but the numeric inputs as well as the symbolic inputs are NamedTuples. I verified that without the change in build_function in this PR, these tests all fail.

@@ -0,0 +1,197 @@
using Symbolics, Test, SparseArrays
Copy link
Member

Choose a reason for hiding this comment

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

this file isn't in runtests.jl so it's not tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I forgot about that. Just did it.

@codecov-commenter
Copy link

codecov-commenter commented Jul 30, 2023

Codecov Report

Merging #929 (4057215) into master (7df5075) will not change coverage.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##           master     #929   +/-   ##
=======================================
  Coverage   77.47%   77.47%           
=======================================
  Files          26       26           
  Lines        3329     3329           
=======================================
  Hits         2579     2579           
  Misses        750      750           
Files Changed Coverage Δ
src/build_function.jl 72.26% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ChrisRackauckas ChrisRackauckas merged commit d0b1724 into JuliaSymbolics:master Jul 30, 2023
14 of 17 checks passed
@raphaelchinchilla raphaelchinchilla deleted the args_as_named_tuples branch July 30, 2023 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants