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

Clean up: remove obsolete functions #87

Closed
fhagemann opened this issue Nov 2, 2020 · 9 comments
Closed

Clean up: remove obsolete functions #87

fhagemann opened this issue Nov 2, 2020 · 9 comments
Milestone

Comments

@fhagemann
Copy link
Collaborator

Some of the functions are still left over from refactoring the JSON files.

One example is the bounding_box function, which returns the crystal_radius and crystal_length which is no longer stored in SolidStateDetector.

Also, we should look for functions that are defined but never called, e.g. from_internal_units.

These functions should be either removed or incorporated accordingly.

@fhagemann fhagemann added this to the v1.0.0 milestone Nov 2, 2020
@lmh91
Copy link
Collaborator

lmh91 commented Nov 2, 2020

Yes, from_internal_units is not really used or just at some points.
At the beginning we did not used Unitful.jl that much.
But we should for external functions. Internally we should convert all the units to SI units using to_internal_units
and afterwards return the output in the units corresponding to the given input units via from_internal_units.

This addresses for example the simulation of events, see #66.

fhagemann added a commit to fhagemann/SolidStateDetectors.jl that referenced this issue Nov 6, 2020
@fhagemann
Copy link
Collaborator Author

How important are the functions get_important_points for the volume primitives?
Apparently, they were used to adapt the grid to the volumes before implementing the painting algorithm.
Are they still required?

@lmh91
Copy link
Collaborator

lmh91 commented Nov 12, 2020

They are extremely important.

fhagemann added a commit to fhagemann/SolidStateDetectors.jl that referenced this issue Nov 12, 2020
@fhagemann
Copy link
Collaborator Author

I came across two functions that are not used and, thus, obsolete (please confirm!)

fhagemann added a commit to fhagemann/SolidStateDetectors.jl that referenced this issue Dec 2, 2020
@fhagemann
Copy link
Collaborator Author

The function _drift_charge(...) in ChargeDrift.jl line 60 seems to be an internal function but is never called. Is it obsolete?

@schustermartin
Copy link
Collaborator

All it does is to put the starting position in a vector and call the _drift_charges(...) function. Basically, the application would be to call the function for a single starting position, warranting the singular, which can be a legit argument for user friendliness. This is an internal function though, so as far as I am concerned it can go.

fhagemann added a commit to fhagemann/SolidStateDetectors.jl that referenced this issue Apr 30, 2021
@fhagemann fhagemann changed the title Clean up: remove obsolte functions Clean up: remove obsolete functions May 8, 2021
lmh91 pushed a commit that referenced this issue Jul 9, 2021
@fhagemann
Copy link
Collaborator Author

I have created a new cleanup branch that we should use to remove obsolete functions and add docstrings to all (exported) functions that remain.

@lmh91
Copy link
Collaborator

lmh91 commented Jul 28, 2021

I think this can be actually closed, right?

@fhagemann
Copy link
Collaborator Author

Yes, I will close this.

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

No branches or pull requests

3 participants