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

Do not aggressively apply dot #136

Merged
merged 3 commits into from
Jul 30, 2023
Merged

Do not aggressively apply dot #136

merged 3 commits into from
Jul 30, 2023

Conversation

lukem12345
Copy link
Member

@lukem12345 lukem12345 commented Jul 30, 2023

Close #134

Currently, when constructing Decapodes, tangent variables are always renamed as the variable they depend on appended with a dot.
E.g. D == \partial\_t(C) creates a Decapode with two variables named C\dot and C.

This strategy was chosen by default since users sometimes only describe TVars in terms of their relation to other quantities. E.g. in \partial\_t(Foo) == Bar(Buzz), the quantity \partial\_t(Foo) is never given a human-legible name. So defaulting to Foo\dot is appropriate.

This is, of course, always fine from a compilation aspect, since the generated program is name-agnostic.

But, this can seriously impact usability. If a user has created a Decapode, and has given a TVar a human-readable name, then they expect to be able to refer to the TVar by that when trying to compose with it.

It should be noted that renaming variables using the append-dot method is an example of where we can make physical systems more legible through analysis of a Decapode. In other words, we can perform some rewriting to tell the user that a variable is a first, second, or higher-order derivative. Users in practice essentially must traverse the TVar table to glean this information themselves. If done elegantly, this exemplifies the automation of physical intuition that we often aim for. If done inelegantly - by renaming too aggressively - we confuse the reader when they try to compose Decapodes.

So, in this PR, we default to using human-legible variable names when provided, and only rename with dots when no name is available. We also offer a function that aggressively performs renaming that the user can call on-demand.

@lukem12345 lukem12345 self-assigned this Jul 30, 2023
@codecov
Copy link

codecov bot commented Jul 30, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.30% 🎉

Comparison is base (335818f) 88.40% compared to head (e0c3b9c) 88.70%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #136      +/-   ##
==========================================
+ Coverage   88.40%   88.70%   +0.30%     
==========================================
  Files           9        9              
  Lines        1009     1036      +27     
==========================================
+ Hits          892      919      +27     
  Misses        117      117              
Files Changed Coverage Δ
src/Decapodes.jl 100.00% <ø> (ø)
src/decapodeacset.jl 96.94% <100.00%> (+0.40%) ⬆️

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

@lukem12345
Copy link
Member Author

@jpfairbanks Just requested your review. find_deps_and_order could be re-written more elegantly. Not sure what the best Julia idiom is for “call function until output does not change”

@jpfairbanks
Copy link
Member

I'm guessing there is something for fixed point iteration, but I haven't used it.

Your algorithm seems reasonably efficient. Since this is enumerating the tangent variables, it should be fast enough.

src/decapodeacset.jl Outdated Show resolved Hide resolved
@lukem12345 lukem12345 merged commit b61ab97 into main Jul 30, 2023
9 checks passed
@lukem12345 lukem12345 deleted the llm/dot_rename branch July 30, 2023 23:38
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.

Bug in Open for variables that are derivatives
2 participants