cfs-energy / cfspopcon

POPCONs (Plasma OPerating CONtours)
https://cfspopcon.readthedocs.io/en/latest/
MIT License
22 stars 14 forks source link

Infra cleanup part2 #41

Closed tbody-cfs closed 4 months ago

tbody-cfs commented 5 months ago

@nelsonand This is a good example for the refactoring that I need. Eventually, I'm hoping that we get rid of the algorithms folder entirely, and provide some better structure to formulas

tbody-cfs commented 5 months ago

TODO

tbody-cfs commented 5 months ago

Comments carried over from #40

tbody-cfs commented 5 months ago

@nelsonand and @aarooshgr. This is part 2 of the refactor. If you get a chance to try this out and let me know if there are confusing bits, that would be greatly appreciated!!

tbody-cfs commented 4 months ago

@hassec When building the documentation with autoapi, I get these errors. Not sure how to fix/if we should.

reading sources... [100%] autoapi/index
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/index.rst:46: WARNING: duplicate object description of cfspopcon.formulas.scrape_off_layer.two_point_model.solve_target_first_two_point_model, other instance in autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/solve_target_first_two_point_model/index, use :no-index: for one of them
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/index.rst:70: WARNING: duplicate object description of cfspopcon.formulas.scrape_off_layer.two_point_model.solve_two_point_model, other instance in autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/solve_two_point_model/index, use :no-index: for one of them
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/solve_target_first_two_point_model/index.rst:4: WARNING: duplicate object description of cfspopcon.formulas.scrape_off_layer.two_point_model.solve_target_first_two_point_model, other instance in autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/index, use :no-index: for one of them
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/solve_two_point_model/index.rst:4: WARNING: duplicate object description of cfspopcon.formulas.scrape_off_layer.two_point_model.solve_two_point_model, other instance in autoapi/cfspopcon/formulas/scrape_off_layer/two_point_model/index, use :no-index: for one of them
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/plotting/index.rst:75: WARNING: duplicate object description of cfspopcon.plotting.make_plot, other instance in autoapi/cfspopcon/plotting/make_plot/index, use :no-index: for one of them
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/plotting/make_plot/index.rst:4: WARNING: duplicate object description of cfspopcon.plotting.make_plot, other instance in autoapi/cfspopcon/plotting/index, use :no-index: for one of them
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
copying assets... done
writing output... [100%] index
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/algorithm_class/index.rst:51: WARNING: py:class reference target not found: LabelledReturnFunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/algorithm_class/index.rst:71: WARNING: py:class reference target not found: LabelledReturnFunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/algorithm_class/index.rst:102: WARNING: py:class reference target not found: GenericFunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/index.rst:82: WARNING: py:class reference target not found: LabelledReturnFunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/index.rst:102: WARNING: py:class reference target not found: LabelledReturnFunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/index.rst:133: WARNING: py:class reference target not found: GenericFunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/index.rst:474: WARNING: py:class reference target not found: FunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/plotting/index.rst:58: WARNING: py:class reference target not found: matplotlib.pyplot.Axes
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/plotting/make_plot/index.rst:43: WARNING: py:class reference target not found: matplotlib.pyplot.Axes
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/unit_handling/decorator/index.rst:41: WARNING: py:class reference target not found: FunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/unit_handling/index.rst:54: WARNING: py:class reference target not found: FunctionType
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/unit_handling/setup_unit_handling/index.rst:50: WARNING: py:class reference target not found: Params
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/unit_handling/setup_unit_handling/index.rst:50: WARNING: py:class reference target not found: Ret
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/unit_handling/setup_unit_handling/index.rst:50: WARNING: py:class reference target not found: Params
/Users/tbody/Code/cfspopcon/docs/autoapi/cfspopcon/unit_handling/setup_unit_handling/index.rst:50: WARNING: py:class reference target not found: Ret
aarooshgr commented 4 months ago

Hi @tbody-cfs and @hassec, I've added the infra_cleanup_part_2 branch into my own fork. However, when I try to run the command poetry run radas -d ./radas_dir, I get "error: extension 'fortran_file_handling' has Fortran sources but no Fortran compiler found." I have downloaded g77, but the error still pops up. Is there a way that this issue can be fixed? Thanks.

aarooshgr commented 4 months ago

Hi @tbody-cfs and @hassec, I've added the infra_cleanup_part_2 branch into my own fork. However, when I try to run the command poetry run radas -d ./radas_dir, I get "error: extension 'fortran_file_handling' has Fortran sources but no Fortran compiler found." I have downloaded g77, but the error still pops up. Is there a way that this issue can be fixed? Thanks.

So it turns out that I was able to solve this issue by installing gfortran (running brew install gcc on my Mac) before running poetry run radas -d ./radas_dir. I think this step would be a good thing to add to the "Getting Started" documentation.

Also, I really like the new code layout! Much easier to work with and navigate, especially when it comes to writing new algorithms like my mcrf_from_fixed_P_sol algorithm.

hassec commented 4 months ago

Sorry @aarooshgr I was travelling for work and missed the above question. Glad to see you managed to fix it.

Also, I really like the new code layout! Much easier to work with and navigate, especially when it comes to writing new algorithms like my mcrf_from_fixed_P_sol algorithm.

Happy to hear that! 👍

ThomasWangAPAM commented 4 months ago

@nelsonand and @aarooshgr. This is part 2 of the refactor. If you get a chance to try this out and let me know if there are confusing bits, that would be greatly appreciated!!

I also got a chance to play with this new layout when I rebase #30. I think the extraction of algorithms from formulas simplifies my work, and the categorization of formulas guides me in terms of where I should introduce the new features. As far as my experience with #30 can tell, this is an effective clean-up for developers. 👍