Skip to content
This repository was archived by the owner on Jun 11, 2026. It is now read-only.

Rename module entry points#947

Merged
dcrc2 merged 3 commits into
masterfrom
dcrc2/rename-module-entry-points
Jul 13, 2021
Merged

Rename module entry points#947
dcrc2 merged 3 commits into
masterfrom
dcrc2/rename-module-entry-points

Conversation

@dcrc2

@dcrc2 dcrc2 commented Jul 13, 2021

Copy link
Copy Markdown
Contributor

This PR renames the pybind entry points to be forward and backward entry and entry_vjp irrespective of whether LM AD is used. (Previously the backward entry point was either rev_entry or sufrev_entry.)

In particular, for the embedded C++ examples, this means that any two C++ functions can be used as the entry points. I've removed the generate_lm parameter from cpp_string_to_autograd_function, which didn't really make sense in this context as nothing is being generated.

In order for this to work, the embedded C++ examples need to have C++ functions with the correct names (entry and entry_vjp). I've done this here by adding trivial forwarding functions with the correct name, rather than by changing the existing names. This is done with PRs #925 and #931 in mind: the forwarding function will need to do a little more work when those PRs are merged. I'm intending to rebase those PRs onto this one.

Note that these changes mean that we no longer generate an entry point for [fwd f] when LM AD is requested. I don't think we were actually using this anywhere.

@dcrc2
dcrc2 requested review from awf and toelli-msft July 13, 2021 10:48
@awf

awf commented Jul 13, 2021

Copy link
Copy Markdown
Contributor

I've just realised I've named these badly, sorry.

"Forward" should really be reserved for "compute the function, and anything else needed for a subsequent backward"

[edited:]
I think now we should call the entry points "func", and "func_vjp". (Or indeed "entry" and "entry_vjp").

Then KscAutogradFunction can decide to make use of them in its "forward" and "backward" pending #768

@dcrc2
dcrc2 force-pushed the dcrc2/rename-module-entry-points branch 2 times, most recently from 479a9f5 to d353737 Compare July 13, 2021 11:50
@dcrc2

dcrc2 commented Jul 13, 2021

Copy link
Copy Markdown
Contributor Author

I think now we should call the entry points "func", and "func_vjp". (Or indeed "entry" and "entry_vjp").

OK, renamed to entry and entry_vjp.

@dcrc2
dcrc2 force-pushed the dcrc2/rename-module-entry-points branch 2 times, most recently from 325a89f to f61b21c Compare July 13, 2021 12:35
@dcrc2 dcrc2 mentioned this pull request Jul 13, 2021

@awf awf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, thanks.

Comment thread src/python/ksc/torch_frontend.py Outdated
@dcrc2
dcrc2 force-pushed the dcrc2/rename-module-entry-points branch from f61b21c to 8afc0c3 Compare July 13, 2021 14:27
@dcrc2
dcrc2 merged commit e232149 into master Jul 13, 2021
@dcrc2
dcrc2 deleted the dcrc2/rename-module-entry-points branch July 13, 2021 14:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants