-
-
Notifications
You must be signed in to change notification settings - Fork 173
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
Add dolfin version to JIT hash #102
Conversation
This is a backport of DOLFIN old commit 1211d17cbf1d35727a57b01c61127bc904becaf1 where it fixed problems on Bamboo
Is this needed? The generated code in FFCX does not include any DOLFIN code. |
This is expression JIT and cpp code JIT, not FFC jit. So it depends on DOLFIN. I'm not saying this is the best solution. Possibly one should hash all the inputs to |
Looks fine to me. Anyway, harmless. |
I don't agree about harmless. I don't believe it is necessary, but will lead to the future 'this must be needed, but I don't know why'. Let's keep it simple. |
I don't know how this should be handled corretly but consider this:
Indeed the compiled module depends on many things and amongst other on dolfin version by depending on |
Ok, this is an example of where the version is needed (at the moment). The plan is to remove support for code like On a wider note, it would be ideal if the auto-generated code (i.e., code generated transparently to the user) has no DOLFIN dependencies. We should aim towards this. |
So, shall we merge or delete this PR? |
This is not clear to me. I don't consider the solution good. The issue is: 1. jitted modules have dependecies, hence 2. dependencies have to be hashed. But hashing dolfin version is not taking care of 2. completely. (I pretty much opened the PR as a bug report, with backport of dolfin-old fix-hack.) |
This is a backport of DOLFIN old commit 1211d17cbf1d35727a57b01c61127bc904becaf1
where it fixed problems on Bamboo.