Skip to content

Diegom spectral function multiprocessing - #80

Merged
mesonepigreco merged 26 commits into
SSCHAcode:masterfrom
diegomartinez2:Diegom_spectral_function_multiprocessing
Oct 18, 2023
Merged

mesonepigreco merged 26 commits into
SSCHAcode:masterfrom
diegomartinez2:Diegom_spectral_function_multiprocessing

Conversation

@diegomartinez2

Copy link
Copy Markdown
Contributor

Fix for the "get_full_dynamic_correction_along_path_multiprocessing" output name.

@mesonepigreco
mesonepigreco self-requested a review October 18, 2023 10:52
@mesonepigreco mesonepigreco added this to the 1.4 milestone Oct 18, 2023
@mesonepigreco

Copy link
Copy Markdown
Collaborator

Hi @diegomartinez2 , is it working? Can I merge in the branch? I would like to release version 1.4

@diegomartinez2

Copy link
Copy Markdown
Contributor Author

There is a conflict with ThermalConductivity.py that I didn't touch.

@diegomartinez2

Copy link
Copy Markdown
Contributor Author

I think is ready now. Maybe not the best solution for the sorting but now should work.

@mesonepigreco

Copy link
Copy Markdown
Collaborator

@diegomartinez2

I think is ready now. Maybe not the best solution for the sorting but now should work.

What is the solution you employed? why do you say it is not the best solution? Just to have a reference for the future.

@mesonepigreco

Copy link
Copy Markdown
Collaborator

This should solve #79

@mesonepigreco

Copy link
Copy Markdown
Collaborator

@diegomartinez2
Does this also fix #77 ? Have you had time to look in that issue? Just want to wrap up and release 1.4 fixing all known bugs.

@diegomartinez2

Copy link
Copy Markdown
Contributor Author

I have not implemented a tested solution for #77 yet.

@diegomartinez2

Copy link
Copy Markdown
Contributor Author

I think #77 can be solved by changing in spectral.py:
From:

# Add all the new computed dynamical matrix
    for iq in range(len(q_tot)):
        new_dyn.dynmats.append(dynq[iq, :, :])

To:

# Add all the new computed dynamical matrix
    new_dyn.dynmats[iq] = dynq[0, :, :]
    for iq in range(1,len(q_tot)):
        new_dyn.dynmats.append(dynq[iq, :, :])

But It needs to be tested.

@mesonepigreco

Copy link
Copy Markdown
Collaborator

I agree,
I merge this branch as soon as the test-suite passes.

@mesonepigreco mesonepigreco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The discussion has been done within the pull request forum.

Pull request approved

@mesonepigreco

Copy link
Copy Markdown
Collaborator

I agree, I made a quick fix in the master. We should add also a test on that function.
Anyway, I will merge it

@mesonepigreco
mesonepigreco merged commit 7458e2b into SSCHAcode:master Oct 18, 2023
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