Update to FST geometry to add a simplified version of the geometry for tracking #147

jdbrice · 2021-09-16T01:50:18Z

No description provided.

update from upstream on Friday, June 11, 2021

Updated StarGeo.xml to be able to acces this in dev2021

…er, heatsinks (draft) )

… rotation of middle disk

This includes material changes

…in PEEK bases, removed unnecessary code

…plified cooling tube connectors)

klendathu2k

Minor issues noted, but no need to hold this.

klendathu2k · 2021-09-16T13:36:01Z

StarVMC/Geometry/FstmGeo/FstmGeo.xml

@@ -424,7 +429,8 @@ The inner support ring (3D printed) should be added
            <var name="rmin"     value="4.1" />
            <var name="rmax"     value="34.00" />
            <var name="dz"       value="30.0" />
-	    <var name="zdisk"    value="{144.633,158.204,171.271}" />
+	<!--    <var name="zdisk"    value="{144.633,158.204,171.271}" /> -->


Legacy positions commented out... may be removed.

Can be removed; maybe changed after survey is understood

klendathu2k · 2021-09-16T13:40:54Z

StarVMC/Geometry/FstmGeo/FstmGeo.xml

 for stainless tube and coolant. Will remove lots of tiny volumes.

-The inner support ring (3D printed) should be added
+The inner support ring (3D printed) should be added (done)


But... volume FSIR is the inner support ring...

klendathu2k · 2021-09-16T13:41:23Z

StarVMC/Geometry/FstmGeo/FstmGeo.xml

@@ -37,7 +43,6 @@ The inner support ring (3D printed) should be added

            FTUO,   <!-- Tube segment for FST half ring outside wedge container -->
            FTUR,   <!-- Tube segment for FST half ring inside wedge container -->
-	    FSIR,   <!-- Inner support Ring -->


... and the inner support ring is removed? Seems inconsistent with the comment above.

When the assembly of the real detector was done the ring had to be removed. For cleanness the note shuld be revised.

klendathu2k · 2021-09-16T14:03:15Z

StarVMC/Geometry/FstmGeo/FstmGeoS.xml

+        -->
+
+
+        <Structure name="mothervol" comment="tubes for the mother volume">


In general, structure naming conventions follow the naming convention for volumes. Four letters, case insensitive. This is due to FORtran memory management constraints (aka ZEBRA). So...

If you only need this geometry for reconstruction, no action is required.

If you think you would ever run simulations with this geometry, then minimally each structure should start with four unique characters.

Are you sure that these data structures need to be 4 characters? I am pretty sure this geometry works in starsim, since that what was used in the last 1.5 year by me for checking it out. In the FstmGeo.F the structure variable do show up with the full name length.

klendathu2k · 2021-09-16T14:04:40Z

StarVMC/Geometry/FstmGeo/FstmGeoS.xml

+            <var name="z(2)"      type="float" />
+        </Structure>
+
+        <Structure name="outerhybrida" comment="outer hybrid a">


Outer and inner hybrids A vs B is where starsim would have a problem. If you think you might want to run a simulation based on this geometry, you should consider renaming these structures.

klendathu2k · 2021-09-16T14:06:46Z

StarVMC/Geometry/StarGeo.xml

    <Construct sys="stgm" config="STGCv1" />
    <!-- Construct sys="etof" config="ETOFv14f"  -->
    <Construct sys="wcal" config="WCALv1" simu="1" />
    <Construct sys="hcal" config="HCALv1" simu="1" />
    <Construct sys="plat" config="PLATon" simu="2" />
-    <Construct sys="pres" config="PRESv1" simu="2" /> 
+    <Construct sys="pres" config="PRESof" simu="2" /> 


Flemming reminded me that preshower detector is not planned for running.

klendathu2k · 2021-09-16T14:07:50Z

StarVMC/xgeometry/.gitignore

@@ -0,0 +1 @@
+*.age


Not quite (there is one file which should not be ignored) but I'll fix after the PR is accepted.

plexoos

As a general comment, you probably don't want to create PRs from main of your fork. If you want to keep your fork in sync with this primary repo, it will be much easier for you to do if you create PRs from a different branch

plexoos · 2021-09-16T14:18:51Z

StarVMC/Geometry/FstmGeo/FstmGeo.xml

@@ -15,19 +15,25 @@ Januar 14, 2021 F.Videbaek
 There is a bit of missing material at the outer edge of the inner mech structure. I will adjust the outer radiues so the toal mateerial should be ok.(approximately
 The outer radius could be make slightly bigger to adjust this.

-To do: Reve the inner novec part of cooling tube and replace with average material
+To do: Reviw the inner novec part of cooling tube and replace with average material


plexoos · 2021-09-17T11:39:08Z

StarVMC/Geometry/FstmGeo/FstmGeo.xml


+July 2021
+It is nesc to move FSTmodules out by 2inch.


It is necessary (?) ....

plexoos · 2021-09-17T11:46:32Z

StarVMC/Geometry/FstmGeo/FstmConfig.xml

+    <Setup  name   = "FSTMv1s"
+	    comment= "Simplified configuration for the Forward Silicon Tracker"
+	    module = "FstmGeoS" 
+	    onoff  = "on"
+            simu   = "2"
+	    >     
+
+    </Setup>


Can you use spaces for indentation consistently?

plexoos · 2021-09-17T11:49:44Z

StarVMC/StarGeometry/.gitignore

+*.h
+*.cxx


Let's see if #148 works. If it does we won't be polluting the checked out source with generated files. In any event, we should modify such files in a separate PR outside of this one

fvidebaek · 2021-09-18T13:26:04Z

StarVMC/Geometry/FstmGeo/FstmGeoS.xml

+
+            FTUO,   <!-- Tube segment for FST half ring outside wedge container -->
+            FTUR,   <!-- Tube segment for FST half ring inside wedge container -->
+	    FSIR,   <!-- Inner support Ring -->


chnge indent

klendathu2k · 2021-09-28T14:18:35Z

I believe the only open questions here involve whitespace and typos in comments. Could we either fix these non-issues, or approve and move forward?

plexoos · 2021-09-28T14:31:19Z

Yes, at this point we should just step in and fix the issues. If I don't see any corrections submitted here, I'll go ahead and fix it later today

fvidebaek · 2021-09-28T15:19:00Z

Hi I have been quite busy the last few weeks, sorry I did not complete. Will try to do tit this afternoon. Flemming

…

On 2021-09-28 10:18, klendathu2k wrote: I believe the only open questions here involve whitespace and typos in comments. Could we either fix these non-issues, or approve and move forward? -- You are receiving this because your review was requested. Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. Triage notifications on the go with GitHub Mobile for iOS [3] or Android [4]. Links: ------ [1] #147 (comment) [2] https://github.com/notifications/unsubscribe-auth/AUIALBDU5VAYS2KX5OR77KLUEHFENANCNFSM5EDVMUMQ [3] https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!P4SdNyxKAPE!R2pTy3l5iAHWIin171Kmol768xnJUW66lFzY_8DX5w0-1-Yf5sDNaYLHISuopFMO$ [4] https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!P4SdNyxKAPE!R2pTy3l5iAHWIin171Kmol768xnJUW66lFzY_8DX5w0-1-Yf5sDNaYLHIQ66956N$

-- Flemming Videbaek senior scientist videbaek @ bnl.gov Brookhaven National Lab Physics Department Bldg 510D Upton, NY 11973 phone: 631-344-4106 cell : 631-681-1596

fvidebaek

I am fine with this

genevb · 2021-09-30T04:37:03Z

AutoBuld failed tonight:

.sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:75:12: error: 'FSTM::FSTMv1s' has not been declared void FSTM::FSTMv1s::setup() { ^ .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:81:12: error: 'FSTM::FSTMv1s' has not been declared bool FSTM::FSTMv1s::list() { ^

.sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx: In function 'bool list()': .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:82:18: error: 'name' was not declared in this scope LOG_INFO << name() << " " << module() << " simu = 2" << " [" << comment() << "]" << endm; ^ .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:82:37: error: 'module' was not declared in this scope LOG_INFO << name() << " " << module() << " simu = 2" << " [" << comment() << "]" << endm; ^ .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:82:74: error: 'comment' was not declared in this scope LOG_INFO << name() << " " << module() << " simu = 2" << " [" << comment() << "]" << endm; ^ .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx: At global scope: .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:88:17: error: 'FSTM::FSTMv1s' has not been declared AgModule* FSTM::FSTMv1s::construct() { ^ .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx: In function 'AgModule* construct()': .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:89:42: error: 'FSTM::FSTMv1s' has not been declared LOG_INFO << "Construct module " << FSTM::FSTMv1s::module() << endm; ^ .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:91:32: error: 'New' was not declared in this scope AgModule* _module = New(); ^ In file included from ./StarVMC/StarAgmlLib/AgBlock.h:17:0, from ./StarVMC/StarAgmlLib/AgModule.h:4, from ./StarVMC/StarGeometry/FstmGeo.h:17, from ./StarVMC/StarGeometry/FstmConfig.h:3, from .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:1: ./StarVMC/StarAgmlLib/AgMath.h: At global scope:
./StarVMC/StarAgmlLib/AgMath.h:15:16: warning: 'twopi' defined but not used [-Wunused-variable] const Double_t twopi = 2.0 * pi; ^ ./StarVMC/StarAgmlLib/AgMath.h:21:16: warning: 'degrad' defined but not used [-Wunused-variable] const Double_t degrad = TMath::Pi() / 180.0; ^ cons: *** [.sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.o] Error 1 cons: errors constructing .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.o

jdbrice and others added 16 commits June 11, 2021 10:16

Merge pull request #1 from star-bnl/main

8c7e629

update from upstream on Friday, June 11, 2021

Initial readme on how to sparse checkout StarVMC

e87cad9

add .gitignore for working directories StarGeometry and xgeometry

2f82fa5

edit description comment

a2389bc

Rotated middle disk by 30 deg to mactch actual installation.

84aeac3

simplified base file for fst geometry

b733202

Updated StarGeo.xml to be able to acces this in dev2021

Simplified FST geometry v1.1 ( cooling tubes, ribs, cooling tube hold…

09e8861

…er, heatsinks (draft) )

Adjustments to commented out simplified heatsinks and included 30 deg…

7f5f220

… rotation of middle disk

Fixed syntax issue with middle disk rotation

683fb5d

Updated material definition for combined stainless and novec

81767a7

Merge branch 'main' of https://github.com/jdbrice/star-sw-1 into main

d6cf073

This includes material changes

Added 3 more cooling tubes, compressed heatsinks into single volumes …

ac48891

…in PEEK bases, removed unnecessary code

Compressed heatsinks and added 3 additional cooling tube volumes (sim…

1b9d23a

…plified cooling tube connectors)

accept gavins changes. removed some dummy not used lines

b080a96

Ring removed.

2e7edd2

remove README for pull request

a46a8cf

jdbrice requested a review from klendathu2k as a code owner September 16, 2021 01:50

klendathu2k approved these changes Sep 16, 2021

View reviewed changes

plexoos added the geometry Anything related to STAR detector geometry label Sep 17, 2021

plexoos reviewed Sep 17, 2021

View reviewed changes

klendathu2k requested a review from fvidebaek September 17, 2021 13:53

fvidebaek reviewed Sep 18, 2021

View reviewed changes

fvidebaek approved these changes Sep 28, 2021

View reviewed changes

Fixes for PR

0d70d9e

plexoos merged commit 802ea05 into star-bnl:main Sep 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to FST geometry to add a simplified version of the geometry for tracking #147

Update to FST geometry to add a simplified version of the geometry for tracking #147

jdbrice commented Sep 16, 2021

klendathu2k left a comment

klendathu2k Sep 16, 2021

fvidebaek Sep 28, 2021

klendathu2k Sep 16, 2021

klendathu2k Sep 16, 2021

fvidebaek Sep 17, 2021

klendathu2k Sep 16, 2021

fvidebaek Sep 17, 2021

klendathu2k Sep 16, 2021

klendathu2k Sep 16, 2021

klendathu2k Sep 16, 2021

plexoos left a comment

plexoos Sep 16, 2021

plexoos Sep 17, 2021

plexoos Sep 17, 2021

plexoos Sep 17, 2021

fvidebaek Sep 18, 2021

klendathu2k commented Sep 28, 2021

plexoos commented Sep 28, 2021

fvidebaek commented Sep 28, 2021 via email

fvidebaek left a comment

genevb commented Sep 30, 2021

		-->


		<Structure name="mothervol" comment="tubes for the mother volume">

		@@ -0,0 +1 @@
		*.age

		*.h
		*.cxx

Update to FST geometry to add a simplified version of the geometry for tracking #147

Update to FST geometry to add a simplified version of the geometry for tracking #147

Conversation

jdbrice commented Sep 16, 2021

klendathu2k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plexoos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klendathu2k commented Sep 28, 2021

plexoos commented Sep 28, 2021

fvidebaek commented Sep 28, 2021 via email

fvidebaek left a comment

Choose a reason for hiding this comment

genevb commented Sep 30, 2021

.sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:75:12: error: 'FSTM::FSTMv1s' has not been declared void FSTM::FSTMv1s::setup() { ^ .sl73_gcc485/obj/StarVMC/StarGeometry/FstmConfig.cxx:81:12: error: 'FSTM::FSTMv1s' has not been declared bool FSTM::FSTMv1s::list() { ^