dschick / udkm1Dsim

A Python Simulation Toolkit for 1D Ultrafast Dynamics in Condensed Matter
http://udkm1dsim.readthedocs.io
MIT License
21 stars 16 forks source link

Problem with N-temperature model and thermal conductivity #80

Closed theodoho closed 1 year ago

theodoho commented 3 years ago

Hi,

I'm trying to build a 2T-model where the electronic thermal conductivity is proportional to the ratio between the electron and lattice temperatures. However, this gives an indexing error. Is there a way around this?

image image image

dschick commented 3 years ago

Hi, this problem is already addressed in #70 It is actually not a big problem to allows to access all subsystem temperatures also in the thermal_conductivity, since this is already done in the sub_system_ coupling. It is more an issue of backwards compatibility and simplicity e.g. for 1TM.

I would like to think of a way to automatically detect if the temperature is input as a scalar (only current subsystem) or as a vector (also accessing other subsystems)

theodoho commented 3 years ago

Got it.

I've noticed that some models assume ke=k0= constant.

I think I'll use a constant value for Tl in the meantime then (e.g. the average).

dschick commented 3 years ago

I just created a PR #81 You could try test it and if it is working fine, I ccan merge it

theodoho commented 3 years ago

This seems to work as far as I can tell. Thank you. :)

dschick commented 3 years ago

Great, could you confirm, that it is working for both cases: scalar vs vectorized temperature dependence?

I would also guess, that it might cost a bit of computational time, since I catched an exception at a very low level of the ode-solver. Maybe you could also compare the computational time for the current development branch vs this fix?

theodoho commented 3 years ago

It seems to work for both cases, and the computational time is similar.

dschick commented 3 years ago

I confirm that it is working. I am closing this one. Please follow my comments in #81

AvonReppert commented 2 years ago

I was just confused. For this to work I needed to update udkm1Dsim from 1.5.0 to version 1.5.3 via github via pip install git+https://github.com/dschick/udkm1Dsim.git

And then this fieature works:

SRO.therm_cond = ['lambda T: (91-9.6)*(T_0/T_1)',
                  5.72*u.W/(u.m*u.K)]

Thanks for implementing it

theodoho commented 2 years ago

Glad to hear you found the issue! :)

/Theodor

On Tue, Dec 7, 2021 at 11:29 PM AleksUDKM @.***> wrote:

I was just confused. For this to work I needed to update udkm1Dsim from 1.5.0 to version 1.5.3 via github via pip install git+https://github.com/dschick/udkm1Dsim.git

And then this fieature works:

SRO.therm_cond = ['lambda T: (91-9.6)(T_0/T_1)', 5.72u.W/(u.m*u.K)]

Thanks for implementing it

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dschick/udkm1Dsim/issues/80#issuecomment-988307548, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIYOCGT5LHMJSKD7PMPDDJ3UP2DERANCNFSM4ZKFD2UA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

dschick commented 2 years ago

Hi @AleksUDKM,

thanks for comming back to this issue. I guess it is not too well documented and the current implementation can lead to some confusions, despite of your version conflict.

Actually, I spend quite some time with thinking about the optimal nomenclature of this feature, as some properties require only one and others all sub_system temperatures. I also wanted to avoid breaking the backwards compatibility with older scripts, e.g. by forcing the user to always use the indices _N, or to overcomplicate it. Finally, some of the properties must be also analytically integratable by sympy which makes things even more complex.

Right now, I try to catch different versions directly in the odefunc of the heat class, which is not too smart. I guess it would be better to internally use always the indexing _N but to allow also for the old indexing style [N] as well as no indexing at all (only use current sub_system temperature) within the user interface.

Let me check if I can go for something in this direction.

dschick commented 1 year ago

I am closing it for now, but please reopen, if you encounter any further issues.