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

Update 1D heateq example #102

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bgroenks96
Copy link
Contributor

  • Fixes a bug in the enthalpy inverse function
  • Initial implementation of a benchmarking loop for comparing adjoint performance

TODO: why is checkpointing always slower and requiring more allocations than no checkpointing??

- Fixes a bug in the enthalpy inverse function
- Initial implementation of a benchmarking loop for comparing adjoint performance

TODO: why is checkpointing always slower and requiring more allocations than no checkpointing??
@bgroenks96
Copy link
Contributor Author

bgroenks96 commented Apr 24, 2024

@ChrisRackauckas and @facusapienza21 Could you guys tell me if I am doing something stupid here? For some reason, enabling checkpointing with InterpolatingAdjoint seems to always increase the runtime and allocations by roughly a factor of 2. Maybe there is some kind of issue with my benchmarking setup? Or maybe there is a type instability issue with the checkpointing implementation?

I separated the forward solve from the adjoint calculation using adjoint_sensitivities to make sure we isolate only the adjoint calculation... but this doesn't seem to change much.

Note that I reduced the size of the problem to make testing easier. It's now only ~150 MOL equations. This can be easily adjusted with the grid_edges variable.

Sorry for the messy code... still a WIP.

@bgroenks96 bgroenks96 marked this pull request as draft April 24, 2024 13:20
@facusapienza21
Copy link
Member

@bgroenks96 What do you want to do with this PR?

@bgroenks96
Copy link
Contributor Author

I guess just close it? It's a shame that it didn't make it into the paper, but I unfortunately don't really have the time (or motivation) to dig further into the details of benchmarking checkpointing 🤷‍♂️

@bgroenks96 bgroenks96 closed this Jun 26, 2024
@bgroenks96
Copy link
Contributor Author

Oh wait, I forgot that it fixed a bug... maybe we should merge it actually, if possible.

@bgroenks96 bgroenks96 reopened this Jun 26, 2024
@bgroenks96 bgroenks96 marked this pull request as ready for review June 26, 2024 22:23
@bgroenks96 bgroenks96 force-pushed the update-heateq-example branch from adf141a to fe9506d Compare June 26, 2024 22:31
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.

2 participants