use np.sum to compute sum #122
Conversation
There was a problem hiding this comment.
Hi Malinda,
Is it correct to assume this is an automated PR generated from R-CPatMiner?
If so, perhaps you need to fix your tool's repo name substitution, as this is not https://github.com/analysiscenter/batchflow.
One other question, do you know the performance impact of this code? We are now iterating through boxes twice, rather than once, though using much faster summing.
pdftotree/utils/pdf/pdf_parsers.py
Outdated
| for i, b in enumerate(boxes): | ||
| box_len_sum = box_len_sum + b.bbox[2] - b.bbox[0] | ||
| num_char_sum = num_char_sum + len(b.get_text()) | ||
| box_len_sum = np.sum([b.bbox[2] - b.bbox[0] for b in boxes] ) |
There was a problem hiding this comment.
chore: remove the extra space before the final parenthesis
There was a problem hiding this comment.
This is done. Thank you
|
Hi @lukehsiao Yes, you got me. I am an author of the project R-CPATMiner. However, this pull request is not auto-generated by the tool. If it is, I would have avoided this error. This is a human-made error due to not paying attention to properly mentioning the project name. I'm sorry for the mistake. Yes you are correct, however, a number of studies that evaluate the effectiveness of list comprehension and Python for loops make the case for list comprehension over Python for loops in terms of efficiency. The use of both Thanks again! |
Description of the problems or issues
Is your pull request related to a problem? Please describe.
Thank you very much for your excellent work in analysiscenter/batchflow.
I am a graduate student at the University of Colorado-Boulder, studying the best practices of evolving ML codes. From our research, one of the most common evolution best practice in ML code is the migration of loop-based computations to vectorization, since this usually improves performance. We made the following changes in batchflow, which remove the FOR loop and use NumPy APIs. I carefully checked the modification to ensure that it does not break the code. I will gladly contribute. Please help me to merge this.
Does your pull request fix any issue.
A possible performance issue
Description of the proposed changes
Use np.sum to compute sum of elements than using inefficient Python for loops
Test plan
I ran
make testas described in the contribution guide line.