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

Reorganise initialisation and simplify main.py and python API #61

Open
helen-brooks opened this issue Jun 19, 2024 · 1 comment
Open

Comments

@helen-brooks
Copy link
Collaborator

helen-brooks commented Jun 19, 2024

Description

Currently initialisation is distributed between lines of code in main.py (to read a config file) and GeometryMaker.parse_json in geometry_maker.py. This distribution will make it harder to be consistent between use of main.py and in-memory invocation of the API.

Suggested implementation

It is reasonable to have a config.json for run settings, and a geometry.json for parameters. However since the outcome of reading config.json only affects the subsequently instantiated GeometryMaker it might make more sense to internalise these functions, and provide them as arguments to the class.

maker=GeometryMaker("config.json","geometry.json")

These could be optional arguments, for users who simply wish to run with default parameters.

Geometry maker would then require two methods, say read_config and read_geometry_params.
These would be called at the end of GeometryMaker.__init__().

The functions that produce the geometry should really be in one function, personally I'd call this generate, and would be the union of make_geometry and imprint_and_merge. Having done this, the method file_to_merged_geometry is probably obsolete at this point.

There should be one high level export that exports everything that has been requested in the config.

main.py would become extremely simple, just parsing the 2 input json file names from command line, and would just need to call

maker=GeometryMaker(config_file,params_file)
maker.generate()
maker.export()

The usage where the parameters are being changed on the fly would be almost identical, just

maker.change_params(...)
maker.generate()
maker.export()
maker.reset()

(Also I think the high level API should avoid reference to cubit, to future proof against ever changing the back end, so suggest renaming reset_cubit to reset.)

@helen-brooks
Copy link
Collaborator Author

helen-brooks commented Jun 19, 2024

Also, anything that constitutes settings should go in the config file, e.g.
maker.print_parameter_logs = True
maker.track_components = False

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant