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

Infra cleanup part2 #41

Merged
merged 41 commits into from
May 6, 2024
Merged

Infra cleanup part2 #41

merged 41 commits into from
May 6, 2024

Conversation

tbody-cfs
Copy link
Collaborator

No description provided.

@tbody-cfs
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

tbody-cfs commented Apr 4, 2024

TODO

  • use wraps_ufunc on integrate_profile_over_volume
  • use wraps_ufunc on fusion_data
  • tidy up fusion energies calculation

@tbody-cfs
Copy link
Collaborator Author

tbody-cfs commented Apr 5, 2024

Comments carried over from #40

@tbody-cfs tbody-cfs mentioned this pull request Apr 5, 2024
Base automatically changed from infra_cleanup_part1 to main April 5, 2024 20:51
Copy link
Collaborator

@hassec hassec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you'll have to lower the coverage threshold a tiny bit to get the CI to pass.

Docs are currently missing for all formulas.

cfspopcon/formulas/geometry/__init__.py Outdated Show resolved Hide resolved
cfspopcon/formulas/geometry/analytical.py Show resolved Hide resolved
cfspopcon/formulas/geometry/volume_integral.py Outdated Show resolved Hide resolved
cfspopcon/formulas/metrics/__init__.py Outdated Show resolved Hide resolved
cfspopcon/formulas/plasma_current/__init__.py Outdated Show resolved Hide resolved
cfspopcon/formulas/radiated_power/__init__.py Outdated Show resolved Hide resolved
cfspopcon/formulas/scrape_off_layer/__init__.py Outdated Show resolved Hide resolved
cfspopcon/formulas/seeded_radiators/__init__.py Outdated Show resolved Hide resolved
cfspopcon/formulas/zeff_and_dilution/__init__.py Outdated Show resolved Hide resolved
tests/test_regression_against_cases.py Outdated Show resolved Hide resolved
@tbody-cfs
Copy link
Collaborator Author

@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

@tbody-cfs tbody-cfs requested a review from hassec April 12, 2024 04:07
@tbody-cfs tbody-cfs force-pushed the infra_cleanup_part2 branch from c6ffdc1 to 8fa93ad Compare April 14, 2024 06:46
@aarooshgr
Copy link
Contributor

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.

ThomasWangAPAM added a commit to ThomasWangAPAM/cfspopcon that referenced this pull request Apr 19, 2024
ThomasWangAPAM added a commit to aarooshgr/cfspopcon that referenced this pull request Apr 19, 2024
@aarooshgr
Copy link
Contributor

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
Copy link
Collaborator

hassec commented Apr 26, 2024

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
Copy link

@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. 👍

@hassec hassec force-pushed the infra_cleanup_part2 branch from 6fb0e6f to fe8bf72 Compare May 6, 2024 18:11
@tbody-cfs tbody-cfs changed the title Draft: Infra cleanup part2 Infra cleanup part2 May 6, 2024
@hassec hassec merged commit ced9073 into main May 6, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants