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.

Sign up for GitHub

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

Jump to bottom

Support reading cached G4DB cross sections from disk #25

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Support reading cached G4DB cross sections from disk #25

wants to merge 5 commits into from

Conversation

EinarElen
Copy link

First draft at being able to read back a cache from disk. Took some faffing around because CSV parsing with iostreams is unpleasant (and I only just now realized that maybe we could use boost for something?) but I've tested it with the xsec_calc app by

  • Creating a cache attached to the model
  • Fill it when we calculate the XS with ElementXsecCache::get
  • Stream it to a separate file (the output of xsec_calc isn't compatible with the ElementXsecCache::stream format)
  • Create a new cache and read the previous cache's output with it
  • Write the second cache to a separate file
  • Diff

I ran this with an energy range of 0 - 100 GeV and these were the only differences

748c748
< 183,74,74700,8289000278.843637
---
> 183,74,74700,8289000278.843638
755c755
< 183,74,75400,8313209739.499187
---
> 183,74,75400,8313209739.499188
823c823
< 183,74,82200,8538021432.452312
---
> 183,74,82200,8538021432.452313

which are all rounding errors in the last available digit.

Technically, this is sufficient for me (as in it is all I need for implementing a separate process/model with its own slooooow XS calculation). If we'd want to have this be useful in LDMX-sw, we would need some additional features and I'm less sure that this is an urgent problem to solve and it has some concerns from a reproducibility perspective (where does the cache go, what if people mix up caches with different xsec calculation methods etc).

Would love to hear your thoughts on it, Tom (not tagging actively because it is a saturday...)

@EinarElen
First draft at reading CSV
bc3c0a6 
Oh the joys of >> formatting CSV...
@EinarElen EinarElen linked an issue Dec 9, 2023 that may be closed by this pull request
Add support for caching the ElementXsecCache/interpolation entries to disk #24
Open
tomeichlersmith
tomeichlersmith requested changes Dec 11, 2023
View reviewed changes
Copy link
Member

@tomeichlersmith tomeichlersmith left a comment

Choose a reason for hiding this comment

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

This looks good, I have one comment related to the actual parsing and the rest are just style.

The tests are failing because #23 that I've been too lazy to patch. I believe we just need to pin cmake to an old version (e.g. 3.22?) so maybe try that on your next push? If you don't want to (or it doesn't work) that's fine, the 10.2.3 tests still pass and this doesn't actually interact with Geant4 so I'm comfortable not testing the other versions.

src/G4DarkBreM/ElementXsecCache.cxx Outdated Show resolved Hide resolved
src/G4DarkBreM/ElementXsecCache.cxx Outdated Show resolved Hide resolved
include/G4DarkBreM/ElementXsecCache.h Outdated Show resolved Hide resolved
src/G4DarkBreM/ElementXsecCache.cxx Show resolved Hide resolved
@EinarElen
Copy link
Author

I believe this should cover the requested changes!

@tomeichlersmith tomeichlersmith self-requested a review December 27, 2023 14:00
tomeichlersmith
tomeichlersmith approved these changes Dec 27, 2023
View reviewed changes
Copy link
Member

@tomeichlersmith tomeichlersmith left a comment

Choose a reason for hiding this comment

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

Stream it to a separate file (the output of xsec_calc isn't compatible with the ElementXsecCache::stream format)

I think this looks good and I wouldn't change anything about the changed files. I am wondering about the "isn't compatible" comment - I would like the possibility of someone to run the xsec calculator and then use that data as the start of a cache. Is that possible currently? If not, that's probably a bug that I should make an issue for.

@EinarElen
Copy link
Author

With isn't compatible, I meant that the XS was calculated with a different method than the user is currently using!

@tomeichlersmith
Copy link
Member

Ah yes 🤦 because the configuration could be different and there is no way for us to check on the cache-loading side.

That makes sense, thank you

@EinarElen
Copy link
Author

Could we add this to the header of this csv file or something similar?

@tomeichlersmith
Copy link
Member

I wouldn't want to break from CSV so that other tools (e.g. pandas) can load these files out of the box for easier analysis. This limits us to just modifying the current line with the column names.

One option is we could add the method name to the Xsec column name, so Xsec [pb] becomes (for example) Full WW Xsec [pb] or similar. I'm not sure how helpful this would be because this just offloads the problem to some magic strings that need to match special values.

Another option is to add another column specifying the method. I think this is overkill but maybe not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomeichlersmith tomeichlersmith tomeichlersmith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for caching the ElementXsecCache/interpolation entries to disk
2 participants
@EinarElen @tomeichlersmith