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

Add lambda thermalq #362

Merged
merged 8 commits into from
Apr 18, 2024
Merged

Add lambda thermalq #362

merged 8 commits into from
Apr 18, 2024

Conversation

Yurlungur
Copy link
Collaborator

@Yurlungur Yurlungur commented Apr 17, 2024

PR Summary

As discussed with @dholladay00 and @gopsub , this adds lambda to the thermalqs list, allowing, on a per-EOS basis, users to request FillEos to do something with input lambdas.

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by using the make format command after configuring with cmake.
  • Document any new features, update documentation for changes made.
  • Make sure the copyright notice on any files you modified is up to date.
  • After creating a pull request, note it in the CHANGELOG.md file.
  • LANL employees: make sure tests pass both on the github CI and on the Darwin CI

If preparing for a new release, in addition please check the following:

  • Update the version in cmake.
  • Move the changes in the CHANGELOG.md file under a new header for the new release, and reset the categories.

@Yurlungur Yurlungur added the enhancement New feature or request label Apr 17, 2024
@Yurlungur Yurlungur self-assigned this Apr 17, 2024
@jhp-lanl
Copy link
Collaborator

Looks like this breaks the python bindings somehow. I don't think it should be too hard to fix.

 45/46 Test #45: PythonBindings .....................................................................***Failed    0.31 sec
      Start 46: test_fortran_interface

Can you run on gitlab as well just to make sure it doesn't break anything else?

@dholladay00
Copy link
Collaborator

dholladay00 commented Apr 17, 2024

  • you'll need to add this to the docs list. using-eos.rst roughly round line 490
  • python/module.cpp needs lambda attribute added
  • test/python_bindings needs a correction to a test calculation by adding an addition | lambda to the test value

@Yurlungur
Copy link
Collaborator Author

Thanks @jhp-lanl @dholladay00 should be fixed now. Tests running on re-git.

@Yurlungur
Copy link
Collaborator Author

OK not sure why the python bindings are still failing... @rbberger any insight?

@rbberger
Copy link
Collaborator

OK not sure why the python bindings are still failing... @rbberger any insight?

Maybe because lambda is a reserved keyword in Python?

@Yurlungur
Copy link
Collaborator Author

OK not sure why the python bindings are still failing... @rbberger any insight?

Maybe because lambda is a reserved keyword in Python?

Hmm... possible. I'll try do_lambda

@Yurlungur
Copy link
Collaborator Author

That worked! Thanks @rbberger !

Ok tests now all apss here. Checking on re-git as well. Ready for re-review, @dholladay00 @gopsub @jhp-lanl @rbberger

@Yurlungur
Copy link
Collaborator Author

Looks like tests are still failing on Roci due to a failed git clone. (@rbberger is this expected? Maybe related to the issues @dholladay00 reported on the mattermost?) However, everything else is passing. I think it's safe to review this and merge it now.

Copy link
Collaborator

@jhp-lanl jhp-lanl left a comment

Choose a reason for hiding this comment

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

Looks good to me

@Yurlungur Yurlungur merged commit ea9e95f into main Apr 18, 2024
4 checks passed
@Yurlungur Yurlungur deleted the jmm/filleos-lambda branch April 18, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants