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

Removing the last varis that need their C++ destructors called #927

Closed
wants to merge 20 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@bbbales2
Member

bbbales2 commented Jul 5, 2018

Submission Checklist

  • Run unit tests: ./runTests.py test/unit
  • Run cpplint: make cpplint
  • Declare copyright holder and open-source license: see below

Summary:

I'm working on removing the dependency of a bunch of varis on the C++ heap (#565)

This is super preliminary. Mostly interested in how many of the tests I broke.

Intended Effect:

That's in the summary!

How to Verify:

tbd

Side Effects:

tbd

Documentation:

tbd

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Columbia University

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

bbbales2 and others added some commits Jul 5, 2018

Initial swing at removing the dependency on the C++ heap of a bunch o…
…f varis (so they don't need destructed) (Issue #565)
Ajdusted a couple check_ldlt_factor_test cases (Issue 565)
Changed output type of prim vectorD to template
Fixed a segfault in g++ and fixed some tests that used ldlts to not b…
…low up cause the ldlt was being called on a non-positive definite matrix (Issue #565)
Removed chainable_alloc dependency from trace_quad_form, quad_form, a…
…nd quad_form_sym

Added build_double_array_if_necessary helper function to efficiently extract values from vars living on the autodiff stack (I should change this to varis -- I don't think these vars should be living on the autodiff stack)
Spit quad_form and quad_form_sym tests apart into separate files (Issue 565)
Added inline modifier to build_double_array_if_necessary functions fo…
…r multiple translation unit tests (Issue 565)
Added new helper function (build_vari_pointer_array_if_necessary) to …
…manage extracting varis from vars

Modified build_double_array_if_necessary to be as const as possible
Improved trace_quad_form
Removed chainable_alloc from trace_gen_quad_form
Added chainable_alloc indicator to trace_inv_quad_form. It was memory leaking before I think
All for Issue 565
@bbbales2

This comment has been minimized.

Member

bbbales2 commented Aug 4, 2018

@bob-carpenter We should meet and talk about this branch next week. It's not finished, but at least a basic grepping for chainable_alloc says it's all been removed.

I've rewritten quite a lot of stuff. I wanna make sure the design patterns and stuff are all good before I go around tightening the bolts.

@bbbales2 bbbales2 changed the title from WIP -- Ignore for now -- Working on removing the last varis that need their C++ destructors called to Working on removing the last varis that need their C++ destructors called Aug 7, 2018

@bbbales2 bbbales2 changed the title from Working on removing the last varis that need their C++ destructors called to Removing the last varis that need their C++ destructors called Aug 7, 2018

@bbbales2 bbbales2 referenced this pull request Sep 11, 2018

Open

remove var_alloc_stack_ and dependencies #565

4 of 19 tasks complete
@syclik

This comment has been minimized.

Member

syclik commented Sep 13, 2018

@bbbales2, if this isn't ready for review, can we close it?

@bbbales2

This comment has been minimized.

Member

bbbales2 commented Sep 13, 2018

Yeah, I'm not gonna finish it before I head back to SB.

@bbbales2 bbbales2 closed this Sep 13, 2018

@bbbales2

This comment has been minimized.

Member

bbbales2 commented Sep 13, 2018

But for people of the future, look at the comments on #565 to get the status of this unfinished pull req.

@syclik

This comment has been minimized.

Member

syclik commented Sep 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment