-
Notifications
You must be signed in to change notification settings - Fork 4
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
gvolume design #5
Comments
Hi Maurik, The text file has a string for position. It can be:
With an arbitrary number of spaces between the numbers. This is passed directly to clhep, no need for a standard. Also, why not have just ONE call to set the position? This way is simpler, standard, and we do not need to write or maintain alternatives. I thiink that
is intuitive enough to be "the way". I like the idea that the object must know how to present itself for inspection. We should definitely do that. |
Hi Mauri,
Got the gemc/api group thing now. Thanks.
Yes, I know you like to do things as text strings :-). I disagree here. You are most likely trying to do something with numbers not with text strings. One would want to do:
x,y,z = function_calculates_xyz()
my_box.set(‘pos’,(x,y,x))
Why would we force the user to go:
x,y,z = function_calculates_xyz()
str_for_gemc = str(x)+”*cm ”+str(y)+”*cm ”+ str(z)+”*cm “
my_box.set(‘pos’,str_for_gemc)
The in between step is totally a place for making errors. Yes, if you have that string already the class should accept it, but this should not be what we encourage.
And I ask you again, how do you plan to incorporate CLHEP into this project?
Best,
Maurik
… On Aug 16, 2017, at 11:40 AM, Mauri ***@***.***> wrote:
Hi Maurik,
The text file has a string for position. It can be:
"1*mm 2.cm 1.3inch "
With an arbitrary number of spaces within the numbers. This is passed directly to clhep, no need for a standard.
Also, why not have just ONE call to set the position? This way is simpler, standard, and we do not need to write or maintain alternatives.
I thiink that
gvolume.setPosition("1*mm 2.cm 1.3inch ")
is intuitive enough to be "the way".
I like the idea that the object must know how to present itself for inspection. We should definitely do that.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEY_4oiMLYLDvh_0huOwENsXHbX05vbIks5sYw1rgaJpZM4O5Gks>.
|
Hi Maurik, I see your point. No plan to incorporate CLHEP, my argument assumed that we only need to pass strings. I do see the advantage in using numbers as well. However I do not like much:
But it may be unavoidable? Let's think about this a bit more. I would prefer using your unit scheme and perhaps having the functions handle it, very similar to what you were doing. So
already contains units and one could do something like:
where x,y,z already have the units in them. |
Hi Mauri,
I think we disagree, but only a little.
In my scheme, we take complexities like units, and resolve them in the Python class. Now the user does not need to worry about converting units, they only need to worry about making sure they specify the correct ones. I am against forcing the user to make the unit conversion, which I think make the overall code (this class + all user geometry definitions) more complicated and more difficult to maintain and debug. I also feel, fairly strongly, that we need to give the user flexibility on how to specify things. Maurizio may like passing strings, but Maurik hates this, and insist he should pass floats since that is what was drilled into him in religious school: “Thou shalt not pass numbers as strings!”. I am against forcing my own indoctrinations on others through an api :-).
So, yes, we should allow for different formats to input code, and thus leave conversions on the user end to a minimum. Mostly, the api should feel “natural” to a Python user, but also to someone more familiar with C++/Java or Perl.
There is a small risk, but it would be acceptable to set a “default units” in the class, so that you can pass only numbers to the position, dimension etc, and the class knows you intent to use the default units. However, doing so introduces the possibility that there is confusion on what the standard units are, with a possible source of error there. We could make the user choose a standard of units, and then write a warning if this is not done and there are no units specified in a call. This, however, adds additional complication to our design.
Best,
Maurik
… On Aug 16, 2017, at 12:06 PM, Mauri ***@***.***> wrote:
Hi Maurik,
I see your point. No plan to incorporate CLHEP, my argument assumed that we only need to pass strings. I do see the advantage in using numbers as well.
However I do not like much:
gvolume.set([1.,2.,1.3],([“mm”,”cm”,”inch”])
But it may be unavoidable? Let's think about this a bit more. I would prefer using your unit scheme and perhaps having the functions handle it, very similar to what you were doing.
So
x,y,z = function_calculates_xyz()
already contains units and one could do something like:
gvolume.setPosition(x,y,z)
where x,y,z already have the units in them.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEY_4l7xFL1WtkRF7qZ_CIeR8TYrQk5hks5sYxN4gaJpZM4O5Gks>.
|
Hi! Jeff Hi everyone, I'm probably misunderstanding so feel free to straighten me out :) I am wondering why the API needs to understand the geometry representation at all. It seems to me that the purpose of the API is to accept some kind of geometry definition from the user through some python statements and then present that definition to gemc in one ic several convenient formats. The API doesn't have to know what a cm is as long as it can tell gemc (or ROOT or whatever) what it needs to know so IT understands what the user intended. All the API needs to do is to check that it has a valid input. For position, there need to be three valid numbers and three valid units stored in the gVolume container. Maurik's dictionary would do fine to ensure the unit string is something we've heard of. Beyond that I think we leave it to the output generator to present the geometry in the way that its consumer wants. If we start translating everything to a common system within the API then we get to where a user can't recognize their own geometry files because the API has cleverly translated everything to some other system. Also, the container in the API never interacts with anything outside the class directly. So the input can be a single string format for simplicity as Mauri suggested but we can easily add cases to parse variations (like giving one unit for the whole vector) if we decide they are useful. Likewise the format for storing in the container is purely a matter of convenience as no outside consumer will ever see it except through the class output methods that can present it however is needed. Storing numerical values and unit strings makes sense to me but it hardly matters. I guess I'm just trying to say that the container isn't storing a geometry representation. It's storing a set of labels that tell something else how to represent the geometry by way of the output methods. Ok, tell me if I went off the rails. :) this is fun to think about, especially with all of you who actually know what you're talking about. |
Hi Jeff, Maurik, Writing email until Seamus and Jeff join the team on github (will copy this there to save it). My original idea was too much "perl-like", i.e. "just fill tables of string". Although that is working well in perl, it is a bit shortsighted in that it cannot take advantage of the power of python. In clas12 sometimes we had to use ROOT to make calculations and "prepare" some variables, then use them in the perl scripts. This could be done within python directly. For example, "pos" could be a 3-vector object and it could be passed to setPosition. Within the script functions could be used to handle pos and rotations instead of having to parse the infos from string, make calculations, then parse it back into a string to be accepted by gvolume. Example: A detector has a "rot". We want to make a copy of it around phi. One would just have to do rot.rotateZ(deltaPhi) instead of parsing rot, getting the number, and produce a new "rot" string. As Jeff suggested, perhaps the "gvolume" class shouldn't know about this. It is not a geometry representation and in the end it just writes strings. It could still handle the various setPosition(string), setPosition(vector) etc. And the units mechanism could be done outside of gvolume. Mauri |
Hi all, Sorry, but I think I fundamentally disagree. If you want to “just store a bunch of strings”, then you don’t need any of this and you just use a list or dict. I do think that we are storing a geometry object in gvolume. If not, what are we storing there? Mauri has a good example with the rotation, which is why I started using Python instead of the Perl interface. I needed complicated rotations to optimally pack a collection of trapezoids with a fixed, very small, amount of space in between. This sounds like a simple problem, but actually it is not and the rotations required quickly drove me nuts. I needed rotation classes to get this done, and wanted everything is a single script. Perl just could not do this. If the gvolume class knows what it is that it stores, then you can verify that the data makes sense. Otherwise you will be depending on something else to decide if the data is correct, but why spit these two? Also, if you want to output to something other than text tables, say you want to render the results using Blender (has a full Python API, so, yes, we should consider that!), you will need to do all sorts of conversions. So then, why would you write the code so that these conversions are done in the Blender output, and again in the ROOT output, and again in the GDML output, and …. Makes no sense to me. The object itself “knows" it is a geometry and “knows" to present its data in a sensible way to either the user or whatever we use as an output. We can argue what a “sensible way” is, but I am not so convinced that the string format is a universal sensible way to represent a geometry object. That is why I suggest we have other input and output representations. I think I am arguing for a fairly standard OOP design, where you do not care (as Jeff points out) how the data is stored internally in the object, but you have all the convenient tools at hand to put the data there and extract it again, in formats that are appropriate for what you are trying to do. If all you are interested in doing is store strings and then extract strings, I don’t think this project makes sense, at least not for me. To me this project is more about the transformations of geometry representations than the storing of a single geometry representation. Best, |
Everyone is on the team so we can continue the discussion on github. Maurik brings up two issues:
My thoughts:
Maurik, any particular reason why you prefer gvolume to know how to write a specific object? The output objects do not have much to do with gvolume itself. |
Hi Mauri,
I didn’t mean “gvolume writes a MySQL string”. Other than that it should implement the Python __str__(), so that you can ask “what are you?” from the command line, the “sensible representations” are those that are needed so that the class that produces the MySQL output can get the information as a string, and the ROOT output one can get the information as floating point numbers, for instance. Only floating point numbers would be an acceptable output format, but only a string containing a floating point number would not, since the latter would need interpreting and thus leave the possibility for interpretation error. I think we agree here?
Next question: Is it cleaner to have:
1) a “factory” that implements various versions of an abstract .write() method
2) or is it cleaner to have “transformations”, that take one object and return another of different type,
3) or have a set of related classes that can consume and produce gvolume classes?
Assuming that we have the gvolume objects stored in a class geometry_store (which would need to have the materials and all that as well).
For 1), I would not quite know how to produce a useful representation for ROOT. Perhaps Mauri can give a better example of how that particular one would work. Perhaps:
geometry_store.set_factory(ROOTGeometry)
RGeom = geometry_store.produce() # Calls the ROOTGeometry converter to produce the ROOTGeometry object.
For 2), you would have something like:
RGeom = TranformToRoot(gvolume_store)
For 3), you would have something like:
RGeom = ROOTGeometry(gvolume_store)
Each of these would probably lead to subtle differences in implementation, but not really anything too radically different. I would lean toward 3), but don’t have any objections to the other 2. There may be other choices as well.
Note that we want to keep all the code in a Python package, which needs to imported. We would want to avoid forcing the user to import all sub-classes, so that they do not need to have ROOT or MySQL installed to still get functionality. I.e. you can “import PythonGeometry.ROOTGeometry” as a separate component, or “from PythonGeometry import *” to get everything.
Best,
Maurik
… On Aug 16, 2017, at 3:57 PM, Mauri ***@***.***> wrote:
Everyone is on the team so we can continue the discussion on github.
Maurik brings up two issues:
gvolume itself “knows" it is a geometry
2 gvolume to present its data in a sensible way
My thoughts:
other than some internal validation, this means for example to have "position" not to be a string, but a vector, and "rotation" as well. I agree. In fact this could be extended to other descriptor, like "color".
not sure about this. I was thinking of a separate class "gfactory" that knows what a gvolume is, and have an abstract method "write()" that is derived by various factories (ROOT, Json, mysql, etc). The class would have with a vector or a map of gvolumes. This would allow the gvolume and gfactory to be simple and specialized.
Maurik, any particular reason why you prefer gvolume to know how to write a specific object? The output objects do not have much to do with gvolume itself.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEY_4skD5SjFtO26pc7NuIaCVwtStHBiks5sY0mYgaJpZM4O5Gks>.
|
Hi Maurik, Yes we agree on the introspection. Here's a general idea I propose for the gfactory. I will use c++ language cause I do not know python enough, but looks like python can do all this (this won't compile, it is just for illustration).
All that may be behind the scenes, in a "main" hidden from the user, where the gfactory is instantiated. Users will write the loadGVolumes(), which may look like:
the outputType can be passed by command line. Of course a lot of details may have to be ironed out ;-) |
Hi Mauri,
The quasi-code you show here is not quite clear to me. Perhaps because if this, it looks overly complicated, and so I fail to see the utility of it.
First, I want to strongly advocate against a “main hidden from the user”. I really do not think that is good design. I totally agree in providing a template main that a user can expand into their own program, but hiding it gives so many issues. How would a user add a command line switch? How is information passed from user routines to/from main? How are you supposed to debug your code if you are not aware of what main does?
I think that trying to hide “main()” makes everything more complex. We have fairly sophisticated users, and I don’t see they will have any problem with a templated main_code.py
Second, I want to bring up naming all this stuff. We seem to be going with “gvolume”, “gwhatever”. That’s OK. But the entire module will need a name, and it cannot be “python”. The module needs to be named so that it can be properly imported and has a clear namespace.
I used the name “PyGeom” in my package, so you would “import PyGeom” to get the functionality.
How about one of these:
GemcGeom
PyGeometry
Anybody else with a good name?
————————————————————
Now, some questions about your gfactory. I am probably not understanding what you are writing because of the quasi-C++ code rather than quasi-Pyhthon, but here goes.
My understanding of factories is that these are primarily used to create objects in a way that allow additional subclasses (or types) to be created at some later time. So the objects are created in a “factory” a.k.a “virtual constructor”, so your code becomes more flexible. I.e.
“Define an interface for creating an object, but let the subclasses decide which class to instantiate. Factory Method lets a class defer instantiation to subclasses.”
(ref: https://www.codeproject.com/Articles/35789/Understanding-Factory-Method-and-Abstract-Factory <https://www.codeproject.com/Articles/35789/Understanding-Factory-Method-and-Abstract-Factory>)
This is not what we are doing here. We are not using different classes and subclasses, and I don’t think you are proposing to have different sub-classes (or types) of gvolume, or am I missing something. So why this factory approach?
Here are my concerns:
Why have one, how does this help?
I don’t see how the syntax you are proposing helps our project or helps the user that tries to define a geometry. I also don’t see why it is preferred to have everything handled through some nameless global mechanism. It seems to obscure what is going on, and I am not in favor of obscuring things.
How does this factory provide multiple functionalities?
Some factories may simply produce an output file and that is it, but not all of them. The ROOT transform produces a ROOT detector tree. You could use it to draw the detector on the screen, or to a pdf file, right from your Python prompt. How does something like that work with a factory that just implements “write[]()”?
How is a user to use the Python prompt to inspect what is happening inside the user code if we follow your example?
Hiding main() and making a loadGVolumes() that seems to “magically” add volumes to some unnamed store, seems to not allow me to debug from a command line. How do I ask Python: “show me all the volume so far”, “draw the second volume for me using ROOT”, “check the overlap between this volume and that”?
It is here that our Python implementation can be so much more useful than the Perl one.
Why is gfactory.write[“JSON”]() superior to JSONwriter(gvolume_store) ?
Sorry to ask so many questions.
Best,
Maurik
… On Aug 16, 2017, at 9:50 PM, Mauri ***@***.***> wrote:
Hi Maurik,
Yes we agree on the introspection.
Here's a general idea I propose for the gfactory. I will use c++ language cause I do not know python enough, but looks like python can do all this (this won't compile, it is just for illustration).
class gfactory {
private:
vector<gvolume> gvolumes
map<string, writer> write
public:
constructor: gfactory(string outputType) {
initializeWriters() < fills map, e.g. write["ROOT"], write["JSON"] etc
loadGVolumes()
write[outputType]->write() < will write the whole gvolumes in the appropriate format
}
addGVolume(gvolume thisV) {gvolumes.push_back(thisV)}
virtual loadGVolumes() < derived and filled by users, see below
}
All that may be behind the scenes, in a "main" hidden from the user, where the gfactory is instantiated. Users will write the loadGVolumes(), which may look like:
loadGVolumes() {
gvolume thisVolume1(name1, mother1) < lets make a nice constructor
thisVolume1.setPosition(x,y,z)
addVolume(thisVolume1)
gvolume thisVolume2(name2, mother2)
thisVolume2.setPosition(x',y',z')
addVolume(thisVolume2)
moreComplicatedRoutine1()
moreComplicatedRoutine2()
}
the outputType can be passed by command line.
materials, mirrors, optical properties would be members of gfactory as well in the same fashion. If a user is not interested, it can skip that implementation and the virtual base class method will be used instead.
Of course a lot of details may have to be ironed out ;-)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEY_4tvv_ffHcU8v7dEh0c-ElBZ3QwLGks5sY5xkgaJpZM4O5Gks>.
|
Hi Maurik, A small example should answer most of these questions, let me try set it up on a new branch, it may take some time as another project will now require my immediate attention in the next couple of weeks. Regarding the name, the api is mostly but not all about "geometry". There are materials, information relative to hit definition and sensitivity, optical properties and mirror. In the gemc code all this is called "gSystem", but that name seems not enough descriptive for a python module. Other ideas (I don't particularly like any of these) gemcDetector Best, Mauri |
Hi Mauri,
Thank you for providing an example. Looking forward to that.
How about we go with “GemcDetector”? It describes what this is all about well enough.
Best,
Maurik
… On Aug 17, 2017, at 2:11 PM, Mauri ***@***.***> wrote:
Hi Maurik,
A small example should answer most of these questions, let me try set it up on a new branch, it may take some time as another project will now require my immediate attention in the next couple of weeks.
Regarding the name, the api is mostly but not all about "geometry". There are materials, information relative to hit definition and sensitivity, optical properties and mirror. In the gemc code all this is called "gSystem", but that name seems not enough descriptive for a python module.
Other ideas (I don't particularly like any of these)
gemcDetector
gemcSystem
pyGemcDetector
pyGSystem
Best,
Mauri
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEY_4t0yvDB4nM5YVvtyneO_5RFkMe1qks5sZIJDgaJpZM4O5Gks>.
|
Hello Mauri,
Good to get started on this right away.
OK to start with the GVolume class, since that is needed for everything else. I think this is actually not quite a trivial class. This class is actually very important, because this is where you will have most of your interaction when defining your geometry. It thus needs to have all the “bells and whistles” to make such an interaction as flexible as possible.
Here is my philosophy on such an object:
Since the object is essentially a “container” for information (in this case the geometry information), it should behave like one, i.e. the following constructs should work.
gvolume[‘pos’] should return the 3 vector for position and the associated dimensions in 2 lists (or tuples).
gvolume.get(‘pos’) does the same.
gvolume.set(‘pos’,’1mm 2.cm 1.3inch’) # This means we DO need string/unit parsing and conversion.
gvolume.set([1.,2.,1.3],([“mm”,”cm”,”inch”]) # this should do the same, so we use overloading.
gvolume[‘pos’] = ([1.,2.,1.3],([“mm”,”cm”,”inch”]) # This we may need to look at. I don’t know if = can be overloaded.
It also needs to be possible to create the object from a simple string, or from a set of arguments:
gvolume(“name | mother | description | '’1mm 2.cm 1.3inch’ | …..)
gvolume(name=“name”,mother=“mother”,description=“This is a volume”,pos = ’1*mm 2.cm 1.3inch’, ….)
Again, items like “pos”, and “dims” need to be flexible in how you define them, so it can also be pos=[1.,2.,1.3],pos_units=[“mm”,”cm”,”inch”]
The object must know how to present itself for inspection.
Define the str() method, so you can print the object and see what it contains. The output of this string can either be the GEMC txt format, or something more verbose.
The class uses the correct formatting so that Python help(gvolume) gives very useful and meaningful output.
More pedantic documentation is written in a Python notebook, which GitHub is happy to render for you to pdf if you don’t have Jupyter installed.
My current implementation has some, but not all of this. I also had the class itself know how to translate to different formats (i.e. it had a write_sql() to put out an SQL string), which I think in our rewrite we could leave to a separate library, i.e. a MySQL input/output class.
As you can see, I think this class does need to ability to convert units. You would want to store the data in consistent units (whatever is the Gemc standard), but you may want to input an object using inches. Translations between the standard length units and angle units are not complicated, and I have already implemented something we can adapt.
I don’t see how you want incorporate CLHEP into this project. Do you mean to use Geant4Py? (http://geant4.web.cern.ch/geant4/UserDocumentation/UsersGuides/ForApplicationDeveloper/BackupVersions/V9.3/html/apas08.html)
I have not used this, but it could be useful for this project. Using this we can replace all of Gemc with Python :-)
I would prefer to keep the core library as free of non standard Python as possible. For instance, the ROOT code should only be in the ROOT module, so that if ROOT is not installed, everything else is still fully functional. We can argue whether Numpy or Scipy are sufficiently standard, but I would hesitate having a CLHEP or G4 Python binding as a pre-requirement for all this to work.
Note: How do we move this discussion to GitHub so it is archived? Is the current correct location for this project gemc/api/python?
Best,
Maurik
The text was updated successfully, but these errors were encountered: