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

Example for issue #31 #33

Closed
wants to merge 1 commit into from
Closed

Conversation

caowencai
Copy link

Discussion at issue #31
Output_parameters turn out to be all-zero inexplicably when n_fits is
set up to large enough.
In this example, it is bound up with available_gpu_memory_ =
std::size_t(double(free_bytes) * 0.1) in line 14, info.cu

Discussion at issue gpufit#31
Output_parameters turn out to be all-zero inexplicably when n_fits is
set up to large enough.
In this example, it is bound up with available_gpu_memory_ =
std::size_t(double(free_bytes) * 0.1) in line 14, info.cu.al model
@jkfindeisen
Copy link
Collaborator

This would add a model (sum of two exponentials) and increase the portion of the memory that is used. I think the model is a valuable addition but increasing memory usage may not be necessary because in the issue the user info was likely larger than necessary anyway. On the other hand, I don't remember why we restricted the memory usage in the beginning, probably to leave room for other applications. We might want to make this adjustable.

Code needs to be checked but merging should be straightforward.

@jkfindeisen
Copy link
Collaborator

I like this model and it's well documented too. I would include it soon (add a part in the documentation).

For the used GPU memory I just remembered what was the reason to limit it. Under certain conditions it was faster not to use (allocate) a larger portion of the memory. The reason being that memory was not a bottleneck there. However, depending on the problem or the GPU, it could be. A smarter solution would be needed. Maybe instead of just increasing it, only increase it, if it would be not enough otherwise.

Will take care of this.

@jkfindeisen
Copy link
Collaborator

Could I make one or two commits on your branch (integrate in documentation, a Matlab example maybe) that you selected for merging and then merge?

@superchromix
Copy link
Collaborator

We will not be merging user model functions in the master branch at this time.

@jkfindeisen
Copy link
Collaborator

This hasn't been looked at for a long time and likely won't integrate easily. The underlying discussion Criteria for pull requests of new models? and related (Concentrate information about a model and Include runtime compilation of CUDA code for custom function models) haven't been finished. So I would close this PR for the moment with the possibility to re-open it later, if the discussion about how to integrate custom models has ended.

@jkfindeisen
Copy link
Collaborator

Also issue #31 was addressed separately.

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