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

fluka : lattice, merge, reflections and tests #83

Merged
merged 16 commits into from
Aug 21, 2023

Conversation

stewartboogert
Copy link
Member

@stewartboogert stewartboogert commented Aug 8, 2023

  • reflection fixes in g4 to FLUKA conversion
  • all FLUKA code to include tests of writing
  • VTK visualisation bug fix ( M S R -> M R S) so should be S (reflection), R (rotation) and upstream transform M
  • start merging FLUKA registries
  • fix writing of transformations (RecursiveTransform, Transform etc.)
  • File comparison tests for all FLUKA output

@codecov
Copy link

codecov bot commented Aug 13, 2023

Codecov Report

Merging #83 (752ce06) into main (8d98320) will decrease coverage by 3.02%.
The diff coverage is 7.50%.

@@            Coverage Diff             @@
##             main      #83      +/-   ##
==========================================
- Coverage   73.79%   70.78%   -3.02%     
==========================================
  Files         137      138       +1     
  Lines       20483    21530    +1047     
==========================================
+ Hits        15115    15239     +124     
- Misses       5368     6291     +923     
Files Changed Coverage Δ
src/pyg4ometry/visualisation/ViewerBase.py 22.66% <0.00%> (ø)
src/pyg4ometry/convert/geant42FlukaBake.py 2.08% <2.08%> (ø)
src/pyg4ometry/fluka/fluka_registry.py 51.45% <20.00%> (-6.98%) ⬇️
src/pyg4ometry/fluka/material.py 77.05% <20.00%> (-3.57%) ⬇️
src/pyg4ometry/misc/TestUtils.py 57.57% <46.15%> (-42.43%) ⬇️
src/pyg4ometry/fluka/Writer.py 90.90% <85.18%> (-0.90%) ⬇️
src/pyg4ometry/convert/__init__.py 100.00% <100.00%> (ø)
src/pyg4ometry/convert/geant42Fluka.py 89.82% <100.00%> (ø)
src/pyg4ometry/fluka/body.py 89.83% <100.00%> (+5.20%) ⬆️
src/pyg4ometry/fluka/flair.py 92.85% <100.00%> (ø)
... and 1 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@stewartboogert stewartboogert self-assigned this Aug 13, 2023
@stewartboogert stewartboogert added bug Something isn't working enhancement New feature or request labels Aug 13, 2023
@stewartboogert stewartboogert marked this pull request as draft August 13, 2023 23:11
@stewartboogert stewartboogert force-pushed the flukaLatticeAndMerge branch 3 times, most recently from 1174ca5 to a014209 Compare August 17, 2023 15:53
@stewartboogert stewartboogert marked this pull request as ready for review August 17, 2023 16:55
@stewartboogert stewartboogert changed the title Fluka lattice and merge Fluka lattice, merge, reflections and tests Aug 17, 2023
@stewartboogert stewartboogert changed the title Fluka lattice, merge, reflections and tests Fluka : lattice, merge, reflections and tests Aug 19, 2023
@lnevay
Copy link
Contributor

lnevay commented Aug 20, 2023

Generally all good. I think though that creating a duplicate file (the bake one) for the conversion will have a bad outcome... Just now the two huge files are nearly identical, but only because one or two shapes are implement.

How about there's a function for each shape, then we make this a derived class and override only those bits. We have to have different behaviour if we bake in the transforms or not, therefore overriding a function would seem the way.

Or, could each fluka solid take the same input and the transform (when it's baked, the transform is set to None) and the solid constructor has a flag to bake in the transform?

As soon as we apply a fix for the conversion it will have to be done in two places just now.

@stewartboogert
Copy link
Member Author

Completely agree. This PR was.supposed to be about lattice and other fluka functionality and turned into tests and some proof of principle ideas of how to fix the fluka conversion with reflections. So I have written up the change required in #92. I propose we merge this as it is blocking #91 and then fix the two code path problem later. Is this ok @lnevay

@stewartboogert stewartboogert changed the title Fluka : lattice, merge, reflections and tests fluka : lattice, merge, reflections and tests Aug 21, 2023
@stewartboogert stewartboogert merged commit caf7641 into g4edge:main Aug 21, 2023
13 checks passed
@stewartboogert stewartboogert deleted the flukaLatticeAndMerge branch August 21, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Converting geant4 to FLUKA with reflections Fluka reader transforms not working
2 participants