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

Made out place SIA2D compatibile with AD #48

Merged
merged 17 commits into from
Mar 28, 2024

Conversation

vivekag7
Copy link
Contributor

Overview/Summary

  • Addressed the need for compatibility of the out place forward model with Dual Numbers
  • Adressed the need for a function that calculated the ice thickness H from the surface velocity V
  • Added basal sliding to iceflow model
  • Some minor fixes

List of Changes

  • Plotting utility fix: Streamlined plotting utility to be more efficient and usable with other objects like prediction
  • Type Compatibility: Changed the type compatibility of the SIA2D constructor to support a wider range of data types for compatibility with Dual Numbers.
  • Optimization Compatibility: Updated the system for better compatibility with optimizing functions using Dual Numbers.
  • Type Specification Removal: Removed unnecessary type specifications in out of place SIA2D utilities to simplify the codebase and improve performance.
  • Basal sliding: Added Basal sliding to the iceflow model
  • Enforce Positive Values: Added checks to enforce positive values in out-of-place calculations within the SIA2D model.
  • New function: Introduced a utility for calculating H from V, expanding the analytical capabilities of the model.

Testing

-Very small fixes to the tests were necessary for compatibility

Additional Notes

When defining fields in a struct, using Matrix{<:Real} instead of a more restrictive type parameter like SIA2Dmodel{F <: AbstractFloat, I <: Integer} is crucial for ensuring compatibility with dual numbers, that why that is done.

@JordiBolibar
Copy link
Member

Hi @vivekag7, thanks for the PR!

I see the tests are failing, there seems to be an issue with the conda environment and Pandas. @facusapienza21 I think you could be of help here. Do you have an idea why this could be failing?

PyError ($(Expr(:escape, :(ccall(#= /home/runner/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:43 =# @pysym(:PyObject_Call), PyPtr, (PyPtr, PyPtr, PyPtr), o, pyargsptr, kw))))) <class 'ImportError'>
  ImportError("Pandas requires version '3.8.0' or newer of 'tables' (version '3.7.0' currently installed).")

I'm afraid it's not something specific from this PR, more due to some changes in Python packages that are messing up our environment.

🔄 synced file(s) with ODINN-SciML/Sleipnir.jl (ODINN-SciML#49)
@JordiBolibar
Copy link
Member

Once you review the PR in Sleipnir I'll integrate it, make a new release and then we can re-run the tests here.

src/models/iceflow/SIA2D/SIA2D_utils.jl Outdated Show resolved Hide resolved
src/models/iceflow/SIA2D/SIA2D_utils.jl Outdated Show resolved Hide resolved
@vivekag7 vivekag7 changed the title Outplace dual Made out place SIA2D compatibile with AD Mar 28, 2024
@JordiBolibar JordiBolibar merged commit 6ead632 into ODINN-SciML:main Mar 28, 2024
1 check passed
@vivekag7 vivekag7 deleted the OutplaceDual branch April 2, 2024 17:25
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