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

treeset: use eigen instead of sparse13 #2643

Closed
wants to merge 35 commits into from
Closed

Conversation

alkino
Copy link
Member

@alkino alkino commented Dec 14, 2023

No description provided.

Copy link

✔️ 5c5c730 -> Azure artifacts URL

Copy link

✔️ 29c58af -> Azure artifacts URL

Copy link

✔️ 6ce1dcb -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

6 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@bbpbuildbot

This comment has been minimized.

Copy link

✔️ 68dca39 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@alkino alkino force-pushed the cornu/sparse13_treeset branch from 68dca39 to 8794fda Compare June 4, 2024 11:56
@alkino alkino closed this Jun 4, 2024
@alkino alkino reopened this Jun 4, 2024
Copy link

✔️ d52b91e -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@@ -54,7 +55,11 @@ void MatrixMap::alloc(int start, int nnode, Node** nodes, int* layer) {
} else {
jt = start + j - nnode;
}
ptree_[plen_] = spGetElement(_nt->_sp13mat, it, jt);
if (it == 0 || jt == 0) {
ptree_[plen_] = &place_holder;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a joke of sparse13 I can explain that (this is funny)

Comment on lines -675 to +679
spPrint(_nt->_sp13mat, 1, 0, 1);
std::cout << &_nt->_sparseMat << std::endl;
} else {
int i, n = spGetSize(_nt->_sp13mat, 0);
spPrint(_nt->_sp13mat, 1, 1, 1);
for (i = 1; i <= n; ++i) {
std::cout << &_nt->_sparseMat << std::endl;
int n = _nt->_sparseMat->cols();
for (int i = 1; i <= n; ++i) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should we do about that? I'm for removing...

Comment on lines +30 to +33
namespace Eigen {
template <typename _Scalar, int _Options, typename _StorageIndex>
class SparseMatrix;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is installed and use, so if we include <Eigen> we need to install Eigen header too...

void* _vcv; /* replaces old cvode_instance and nrn_cvode_ */
double* _sparse_rhs; /* rhs matrix for sparse13 solver. updates to and from this vector
need to be transfered to actual_rhs */
Eigen::SparseMatrix<double, 1, int>* _sparseMat{}; /* handle to general sparse matrix */
Copy link
Member Author

@alkino alkino Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We give number otherwise we have to declare the enum behind the option and I have no idea how to do that without conflict at resolution...

spSolve(_nt->_sp13mat, _nt->_sp13_rhs, _nt->_sp13_rhs);
_nt->_sparseMat->makeCompressed();
Eigen::SparseLU<Eigen::SparseMatrix<double, Eigen::RowMajor>> lu(*_nt->_sparseMat);
Eigen::Map<Eigen::VectorXd> rhs(_nt->_sparse_rhs + 1, _nt->_sparseMat->cols());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other joke of sparse13 the matrix is 1-indexed so we use the vector off by one.

Comment on lines +499 to +504
Eigen::SparseMatrix<double, Eigen::RowMajor>& m_ = *_nt->_sparseMat;
for (int k = 0; k < m_.outerSize(); ++k) {
for (Eigen::SparseMatrix<double, Eigen::RowMajor>::InnerIterator it(m_, k); it; ++it) {
it.valueRef() = 0.;
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An other funny joke of sparse13 the function spClear just reset all pre-declared number to 0 instead of removing them (it should be like that because this is a sparse matrix...).

Comment on lines 2002 to 2029
const Node* nd = nt->_v_node[in];
const Extnode* nde = nd->extnode;
const Node* pnd = nt->_v_parent[in];
int i = nd->eqn_index_;
nt->_sparseMat->coeffRef(i - 1, i - 1) = 0.;
if (nde) {
for (int ie = 0; ie < nlayer; ++ie) {
int k = i + ie + 1;
nt->_sparseMat->coeffRef(k - 1, k - 1) = 0.;
nt->_sparseMat->coeffRef(k - 1, k - 2) = 0;
nt->_sparseMat->coeffRef(k - 2, k - 1) = 0;
}
}
if (pnd) {
int j = pnd->eqn_index_;
nt->_sparseMat->coeffRef(j - 1, i - 1);
nt->_sparseMat->coeffRef(i - 1, j - 1);
if (nde && pnd->extnode)
for (int ie = 0; ie < nlayer; ++ie) {
int kp = j + ie + 1;
int k = i + ie + 1;
nt->_sparseMat->coeffRef(kp - 1, k - 1) = 0;
nt->_sparseMat->coeffRef(k - 1, kp - 1) = 0;
}
}
}
// nt->_sparseMat->makeCompressed();
for (in = 0; in < nt->end; ++in) {
Copy link
Member Author

@alkino alkino Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy of the code below for allocating values.
sparse13 is more or less a linked list of single value so when we get pointer afterward they are stable.
Eigen is a kind of vector of values. So can be reallocate. So we reserve all the values rights now and get the pointer afterward.

Copy link

✔️ 61aca57 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

Copy link

✔️ 846364d -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

Copy link

✔️ f4bc02d -> Azure artifacts URL

@alkino alkino force-pushed the cornu/sparse13_treeset branch from 3a12308 to f4bc02d Compare June 6, 2024 12:11
Copy link

✔️ f4bc02d -> Azure artifacts URL

Copy link
Contributor

github-actions bot commented Jun 6, 2024

NEURON ModelDB CI: launching for f4bc02d via its drop url

@bbpbuildbot

This comment has been minimized.

Copy link

sonarqubecloud bot commented Jun 6, 2024

Please retry analysis of this Pull-Request directly on SonarCloud

Copy link

✔️ e1df5f2 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

Copy link

Copy link

✔️ 6b411b6 -> Azure artifacts URL

@alkino
Copy link
Member Author

alkino commented Oct 24, 2024

Superseeded by #3142

@alkino alkino closed this Oct 24, 2024
@alkino alkino deleted the cornu/sparse13_treeset branch November 7, 2024 13:23
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