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

Small improvements to dec_p_derivbound #98

Merged
merged 2 commits into from
Jun 12, 2024
Merged

Small improvements to dec_p_derivbound #98

merged 2 commits into from
Jun 12, 2024

Conversation

GeorgeR227
Copy link
Contributor

Removed some intermediate arrays that are not really needed.

Done in dec_p_derivbound
@GeorgeR227
Copy link
Contributor Author

Before:
Operator: Boundary
Variant: New Form-1, [0.169584 ms, 0.382176 MB]
Variant: New Form-2, [0.2345205 ms, 0.43472 MB]

Operator: Dual Derivative
Variant: New Dual-Form-0, [0.2308955 ms, 0.434064 MB]
Variant: New Dual-Form-1, [0.179959 ms, 0.38152 MB]

Operator: Exterior Derivative
Variant: New Form-0, [0.218458 ms, 0.361184 MB]
Variant: New Form-1, [0.2857295 ms, 0.444304 MB]

After:
Operator: Boundary
Variant: New Form-1, [0.147083 ms, 0.365456 MB]
Variant: New Form-2, [0.247791 ms, 0.365376 MB]

Operator: Dual Derivative
Variant: New Dual-Form-0, [0.250625 ms, 0.36472 MB]
Variant: New Dual-Form-1, [0.158458 ms, 0.3648 MB]

Operator: Exterior Derivative
Variant: New Form-0, [0.194375 ms, 0.344464 MB]
Variant: New Form-1, [0.282 ms, 0.37496 MB]

@lukem12345
Copy link
Member

lukem12345 commented Jun 12, 2024

Nice. Are there any changes here on the output of @code_warntype?

This issue feels relevant #83

@GeorgeR227
Copy link
Contributor Author

GeorgeR227 commented Jun 12, 2024

Good catch, the numeric_sign was falling into the generic case caused by the issue you linked (attr_types can't be used for inference). I've type-asserted them to be bools and the problem seems to be resolved now.

@GeorgeR227
Copy link
Contributor Author

Latest:
Variant: New Form-1, [0.142125 ms, 0.365456 MB]
Variant: New Form-2, [0.2445835 ms, 0.365376 MB]

Operator: Dual Derivative
Variant: New Dual-Form-0, [0.249375 ms, 0.36472 MB]
Variant: New Dual-Form-1, [0.154042 ms, 0.3648 MB]

Operator: Exterior Derivative
Variant: New Form-0, [0.193291 ms, 0.344464 MB]
Variant: New Form-1, [0.28525 ms, 0.37496 MB]

@GeorgeR227 GeorgeR227 requested a review from lukem12345 June 12, 2024 19:13
@lukem12345
Copy link
Member

Good catch, the numeric_sign was falling into the generic case caused by the issue you linked (attr_types can't be used for inference). I've type-asserted them to be bools and the problem seems to be resolved now.

Yeah type-asserting here is a good decision because the FastDEC module is allowed to make those sorts of assumptions.

@lukem12345 lukem12345 merged commit 7d61af3 into main Jun 12, 2024
7 checks passed
@lukem12345 lukem12345 deleted the gr/rmv-d-allocs branch June 12, 2024 19:21
KevinDCarlson pushed a commit that referenced this pull request Jul 24, 2024
* Remove intermediate arrays

* Type assert bools
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.

2 participants