Changes in users initialization #102

psauvan · 2024-04-27T17:25:46Z

Changes allow to set user parameters by either using the configuration file (the original one)

from geouned import CadToCsg

GEO = CadToCsg('Conversion to CAD')
GEO.SetConfiguration('config.ini')
GEO.Start()

or setting parameters directly to the GEOUNED instance.

from geouned import CadToCsg

GEO = CadToCsg('Conversion to CAD')
GEO.set('stepFile', 'myCAD.stp')
GEO.set('geometryName', 'CSGmodel')
GEO.set('outFormat', ('mcnp', 'openMC_XML'))
GEO.set('planeDistance', 0.05)
GEO.set('quadricPY', True)
GEO.set('P_abc', '12f')
GEO.set('P_d', '12f')
GEO.Start()

The modifications implemented should resolve the issue #88

…rom dev (GEOUNED-org#92) (GEOUNED-org#93)

Change to allow setting users parameters by either config file or geouned set method

shimwell · 2024-04-28T13:04:43Z

scripts/geounedClassCall.py

+GEO.set('stepFile', stepFileName)
+GEO.set('geometryName','Placa')
+GEO.set('outFormat', ('mcnp', 'openMC_XML'))


I think this would be the normal way to set class attributes that python users would be familiar with

Suggested change

GEO.set('stepFile', stepFileName)

GEO.set('geometryName','Placa')

GEO.set('outFormat', ('mcnp', 'openMC_XML'))

GEO.stepFile = stepFileName

GEO.geometryName = Placa

GEO.outFormat = ('mcnp', 'openMC_XML')

Ideally this would work for options and tolerances as well

shimwell · 2024-04-28T13:06:22Z

src/geouned/GEOUNED/Utils/Options/Classes.py

+class Tolerances:
+    from .tolerancesDefault import defaultValues, typeDict, KwrdEquiv

+    @classmethod
+    def setDefaultAttribute(cls):
+        for key,value in cls.defaultValues.items():
+            setattr(cls, key, value)


We can set all the defaults in the Class init with default args

let me make a PR into this PR to show what I mean

shimwell · 2024-04-28T13:07:39Z

src/geouned/GEOUNED/Utils/Options/Classes.py

+    @classmethod
+    def setAttribute(cls,key,value):
+        if key in cls.defaultValues.keys():
+            setattr(cls, key, value)


classes allow attributes to be set if we set them in the regular python manner without the need for a custom Attribute setter, so I don't think we need this

shimwell · 2024-04-28T13:08:14Z

src/geouned/GEOUNED/Utils/Options/Classes.py

+    from .mcnpNumericDefault import defaultValues
+
+    @classmethod
+    def setDefaultAttribute(cls):
+         for key,value in cls.defaultValues.items():
+             setattr(cls, key, value)
+
+    @classmethod
+    def setAttribute(cls,key,value):
+        if key in cls.defaultValues.keys():
+            setattr(cls, key, value)


Same comments as above for the class Options

shimwell · 2024-04-28T13:08:36Z

src/geouned/GEOUNED/Utils/Options/mcnpNumericDefault.py

@@ -0,0 +1,16 @@
+defaultValues = {


this can be done in the class on creation

shimwell · 2024-04-28T13:09:30Z

src/geouned/GEOUNED/Utils/Options/optionsDefault.py

@@ -0,0 +1,30 @@
+# default options values
+defaultValues = {


Same comment as above, can be moved into the class. Then users know the default values that are being used as the class tells them in the doc strings and with hover text in IDEs

shimwell · 2024-04-28T13:09:51Z

src/geouned/GEOUNED/Utils/Options/tolerancesDefault.py

@@ -0,0 +1,53 @@
+defaultValues = {


same comment as above, we can move this to the class

shimwell · 2024-04-28T13:10:36Z

src/geouned/GEOUNED/Utils/Options/tolerancesDefault.py

+    "min_area": float,           # minimun face area to consider in cell definition
+}  
+
+KwrdEquiv = {


I saw this in the original init file and it would be really great if we just have one set of names and don't need an equivalence dictionary

shimwell · 2024-04-28T13:11:16Z

src/geouned/GEOUNED/__init__.py

+        Options.setDefaultAttribute()
+        McnpNumericFormat.setDefaultAttribute()  
+        Tolerances.setDefaultAttribute() 


these three lines won't be needed if we declare the defaults in the class args

shimwell · 2024-04-28T13:15:00Z

tests/test_convert.py

-
-    with open(output_dir / "config.ini", mode="w") as file:
-        file.write(template)
+    template = {


I notice [Parameters] and [Options] have been removed from the config file, but I don't see this change in the other config files for example those config files in scripts. Perhaps we still need [Parameters] and [Options] in here?

There are parts of the GEOUNED.init.py file that check for Parameters and Options so I guess the options parameters like forceCylinder are not getting set?

[Parameters], [Options] and the other sections identifiers are still required in the external config file when the method SetConfiguration (renamed from SetOptions ) is used to pass the parameters values.
In the tests/test_convert.py I changed the mode to pass the parameters, instead of writing a file and use the SetConfiguration method, all parameters are stored in a dictionnary and passed with the method set. This method is able to identify any valid parameter name (of any section) and assign its value to the corresponding variable/attribute.

Sounds like we now have arguments for parameters and options being added to the CadToCsg class in the same manner as CadToCsg arguments.

shimwell · 2024-04-28T13:17:50Z

@psauvan I think this PR is a great step in a nice direction. Congrats.

I have a few suggestions if you are interested.

I think we can ignore all the code comments I've made above if you want I can follow up this PR with another PR that shows more clearly what I'm trying to express

shimwell

I think this is in general a good step in the right direction, let us merge this in to dev and we can build on it.

shimwell · 2024-04-28T14:09:34Z

Perhaps just one answer on this comment would be good before merging
https://github.com/GEOUNED-org/GEOUNED/pull/102/files#r1582156741

@psauvan

* Add a github workflow that prevents PRs to main that is not comming from dev (#92) * Fix to position of additional cylindrical surface used to limit torus extent (#96) * camel case for all classes (#95) * camel case for all classes * corrected file imports for georeverse * Apply suggestions from code review variable back to snake case * variable back to snake case * back to snake case * back to previous var name * Snake case variables (PR 1 of several) (#98) * snake case for CellDefinitions file, tests pass * vars in snake_case 2 more files * working still * using is to compare with bool (#99) * Add comments to Geometry.py and booleanFunction.py files (#101) * Add comments to Geometry_GU.py * Add comments to booleanFunction.py * Add comments to Geometry_GU.py * Specific imports instead of wild card (#100) * improved import statements flake8 hints * removed extra line * solve issue #75 in getTransMatrix function (#103) * Changes in users initialization (#102) * Add a github workflow that prevents PRs to main that is not comming from dev (#92) (#93) * Add scripting interface Change to allow setting users parameters by either config file or geouned set method * minor fix * Changes in geounedClassCall.py --------- Co-authored-by: AlvaroCubi <[email protected]> * Adding formatter to CI to keep code in PEP8 (#104) * added format checker * run black . locally * f strings instead of .format (#106) * replaced all .format with f strings * corrected f string conversion * Snake case variables (PR 2 of several) (#105) * snake case im more varibles * formatting --------- Co-authored-by: Patrick Sauvan <[email protected]> * Quicker tests for ci (#109) * removing slowest two geometries * removing slow files from CI * format * added draft template (#97) * Removing unused code (#107) @psauvan comment has been taken into account. Unreachable code that was there for future consideration has been changed to comments. * if statement includes False so never runs code * if statement includes True so always run and never reaches else * line after return never reached * unused arg * unused arg * unused arg calling function * commented unused code as requested * formatting * format * Testing writing inputs for more MC codes (#108) * also writting openmc python, phits and serpent files * formatting * decreased lcoal run time and added all outFormats * black using same version as in pyproject * py3.11 in formatting ci * matched exactly black versions * simpler black pinning * format * Snake case functions (#115) * converted more functions * more snake case functions * more snake case function names * more snake case functions * more snake case functions * more snake case functions * formatting * formatting * formatting * formatting * converting function names to snake class * duplicate import openmc (#116) * Sphinx based docs with CI action to build and hosted on gh-pages (#119) * started rtd * testing docs action * update pip * corrected python version * added s to docs * publish every time * added static folder * skipping ci for non code files * changed to rerad the docs theme * started install section * added more sections * added api class * docs ci using conda * added -y for freecad * trying py 3.12 * mimic working conda ci * using rst so no markdown convertion needed * added github edit link * removed path prints * added readme link, badge * format * Add issue templates (#112) * Add a github workflow that prevents PRs to main that is not comming from dev (#92) (#93) * Create feature request template * improve documentation template * update feature request template --------- Co-authored-by: AlvaroCubi <[email protected]> * Trigger docs build on push only (#126) * pushing triggers doics build * added if push * added missing = sign (#130) * added missing package (#135) * replaced print statements with log (#133) * replaced print statements with log * added logger * formatting * removed encoding arg * removed verbose as all gets logged * removed unused logger * removed , from logging cmd * sorted imports * added another tqdm loop * isort * format * Fixing logger commands (#137) * fixed multi comma log statements * capital letters for progress bar sections * formatted * removal of Options as global variable (#138) * instances of Options class passed by arg * updated usage to show how to use Options * format pep8 * added missing options arg * added missing args options * passing in options as arg * bug fix for compsolid * format * removal of Tolerances as global variable (#139) * passing tolerance as arg not global var * added tolerance example to usage section * one set of argument names * moved atr and types getting out of loop * format * passing missing options and tolerances * format * found bug hidden in try: except statement * using tolerance for more local varibles * formatting * Numerical format to normal class (#141) * replaced global with class passed by arg * format * Improving docs install section (#145) * added dev install instructions * added method section * testing the setting of all class arguments (#147) * Adding getters setters type checking to Options, Tolerances, NumericFormat class (#148) * added type ching setters * added low limit checking for reals * format * added type checkers for Tolerances * format * Add settings class (#149) * settings being passed through the code * all the attributes show in usage example * Adding export csg method (#150) * added export csg method * format * leaving comments in tests * Adding support for json config file (#152) * testing from json * returned file to pre PR status * returned file to init * returned file to init * temp fixing for serpent universe, issue raised * making use of stp file already on repo * Adding command line tool including documentation and testing (#155) * split usage into two sections * added minimal config from cli example to ci * improved cli docs * format * removed duplication between readme and docs (#156) * Moving code from __init__ file to core.py (#157) * moved core logic from init * format * Fixing issue 154 (#158) * added test that currently fails * fixed bug with freecad attribute usage * added missing arg * format * added missing args bug (#159) * added missing args * format * added description of solid being decomposed (#161) * Context managers for writing try 2 (#160) * Add a github workflow that prevents PRs to main that is not comming from dev (#92) (#93) * using context managers to write files * review comments 4/5 --------- Co-authored-by: AlvaroCubi <[email protected]> * Creating folder if needed (#162) * added tests to check writing to folder * back to org * back to org * back to org * creating output folder prior to writing * isort on the imports * format * setting line length to 128 (#163) * Lower case folder names and files (#165) * renamed subfolders to lowercase * renamed files to lower case * openmc_py and openmc_xml lower case (#166) * openmc_py and openmc_xml lower case * removed outdated code comment * Added type checking on export csg (#167) * added input checking on export_csg * compacted same types * Update src/geouned/GEOUNED/core.py --------- Co-authored-by: Patrick Sauvan <[email protected]> * Adding windows install instructions (#169) * added windows install instructions * improved install instructions --------- Co-authored-by: teade <[email protected]> Co-authored-by: Jonathan Shimwell <[email protected]> Co-authored-by: Jonathan Shimwell <[email protected]> Co-authored-by: Patrick Sauvan <[email protected]> Co-authored-by: alberto743 <[email protected]> Co-authored-by: Alex Valentine <[email protected]>

AlvaroCubi and others added 4 commits April 26, 2024 13:36

Add a github workflow that prevents PRs to main that is not comming f…

76ef697

…rom dev (GEOUNED-org#92) (GEOUNED-org#93)

Add scripting interface

9b3180b

Change to allow setting users parameters by either config file or geouned set method

minor fix

f7720bb

Changes in geounedClassCall.py

2463ece

psauvan requested a review from AlvaroCubi April 27, 2024 17:25

Merge branch 'GEOUNED-org:main' into dev_initialization

3cd60ea

psauvan mentioned this pull request Apr 28, 2024

[BUG] - conversions inherit previous conversion attributes #88

Closed

shimwell reviewed Apr 28, 2024

View reviewed changes

shimwell approved these changes Apr 28, 2024

View reviewed changes

psauvan merged commit 8e2fd7f into GEOUNED-org:dev Apr 28, 2024
7 checks passed

psauvan deleted the dev_initialization branch May 2, 2024 11:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes in users initialization #102

Changes in users initialization #102

psauvan commented Apr 27, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024

shimwell Apr 28, 2024 •

edited

Loading

psauvan Apr 28, 2024

shimwell Apr 28, 2024

shimwell commented Apr 28, 2024

shimwell left a comment

shimwell commented Apr 28, 2024

Changes in users initialization #102

Changes in users initialization #102

Conversation

psauvan commented Apr 27, 2024

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

shimwell Apr 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shimwell commented Apr 28, 2024

shimwell left a comment

Choose a reason for hiding this comment

shimwell commented Apr 28, 2024

shimwell Apr 28, 2024 •

edited

Loading