Batch processing of Chemtab Source Computation#506
Merged
klbudzin merged 9 commits intoUBCHREST:mainfrom Jan 28, 2025
Merged
Conversation
mmcgurn
requested changes
Dec 19, 2023
Contributor
mmcgurn
left a comment
mmcgurn
left a comment
There was a problem hiding this comment.
I have some questions about the batch processing.
| std::size_t ndata = ninputs * sizeof(float); | ||
| TF_Tensor *input_tensor = TF_NewTensor(TF_FLOAT, dims, (int)ndims, data, ndata, &NoOpDeallocator, nullptr); | ||
| std::size_t ndata = sizeof(input_data); | ||
| TF_Tensor *input_tensor = TF_NewTensor(TF_FLOAT, dims, (int)ndims, input_data, ndata, &NoOpDeallocator, nullptr); |
Contributor
There was a problem hiding this comment.
I think we might be able to save time by reusing the memory/tensor object? We could do that by having the ChemTabSourceCalculator own the tensor. The ChemTabSourceCalculator instance is always the same batch size.
| } | ||
|
|
||
| // Using batch overloaded version | ||
| // TODO: consider/optimize the problem that is likely requires loop to copy these arrays into TF inputs?? |
Contributor
There was a problem hiding this comment.
If the ChemTabSourceCalculator instance owned the tensor, you could copy directly into the tensor here.
src/eos/chemTab.cpp
Outdated
|
|
||
| // Verified to work 11/7/23 | ||
| // NOTE: id arg is to index a potentially batched outputValues argument | ||
| void ablate::eos::ChemTab::extractModelOutputsAtPoint(const PetscReal density, PetscReal *densityEnergySource, PetscReal *densityProgressVariableSource, PetscReal *densityMassFractions, |
Contributor
There was a problem hiding this comment.
Would you mind changing the names to match the google style case? https://ablate.dev/content/development/Formatting.html
src/eos/chemTab.hpp
Outdated
| PetscReal* densityMassFractions) const; | ||
| void ChemTabModelComputeFunction(const PetscReal density[], const PetscReal* const* const densityProgressVariables, PetscReal** densityEnergySource, PetscReal** densityProgressVariableSource, | ||
| PetscReal** densityMassFractions, size_t batch_size) const; | ||
| // Batched Version of above |
Contributor
There was a problem hiding this comment.
Could you change this comment to work with doxygen for the method signature? Is this method used anywhere or tested?
profPlum
force-pushed
the
batch_processing
branch
from
bbbfc48 to
6c7fc2b
Compare
profPlum
force-pushed
the
batch_processing
branch
from
6c7fc2b to
872626c
Compare
+ some helpful utility functions for debugging
essentially passing to dummy batch function which manually calls the unbatched version, but good test case.
the single dummy batched ChemTabModelComputeFunction
chemistry source only for now)
profPlum
force-pushed
the
batch_processing
branch
from
83f4f21 to
3960caa
Compare
klbudzin
approved these changes
Jan 28, 2025
kolosret
pushed a commit
to kolosret/ablate
that referenced
this pull request
Aug 5, 2025
* commiting clarifying comments and & function definitions + some helpful utility functions for debugging * fixed problem with previous commit where consts weren't being used where needed * first draft of dummy batch api changes essentially passing to dummy batch function which manually calls the unbatched version, but good test case. * added dummy batched version of every function, each of which relies on the single dummy batched ChemTabModelComputeFunction * commiting first working draft of batched processing (implemented on chemistry source only for now) * clarified/corrected confusing & outdated comment * commiting reformatted code & version bump * reworked version so this would be a patch increment since it is just optimization * Updated doxygen comments and addressed various style issues with the PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've manually verified it produces the same results as before and it appears to now be twice as fast.