Skip to content

Promote Field Getters to Public API#806

Merged
jeremylt merged 20 commits intomainfrom
jeremy/get-fields
Sep 14, 2021
Merged

Promote Field Getters to Public API#806
jeremylt merged 20 commits intomainfrom
jeremy/get-fields

Conversation

@jeremylt
Copy link
Copy Markdown
Member

@jeremylt jeremylt commented Sep 8, 2021

I also want to add this to the Rust interface to make my mini-app development in Rust easier.

Comment thread doc/sphinx/source/releasenotes.md
Comment thread rust/libceed/src/operator.rs Outdated
Copy link
Copy Markdown
Member

@jedbrown jedbrown left a comment

Choose a reason for hiding this comment

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

Thanks for taking the lead on this.

Comment thread doc/sphinx/source/releasenotes.md
Comment thread interface/ceed-operator.c Outdated
Comment thread interface/ceed-operator.c Outdated
Comment thread interface/ceed-qfunction.c Outdated
Comment thread interface/ceed-qfunction.c Outdated
Comment thread rust/libceed/src/qfunction.rs Outdated
Comment thread rust/libceed/src/operator.rs Outdated
@jeremylt
Copy link
Copy Markdown
Member Author

jeremylt commented Sep 9, 2021

The Julia CI issue seems to be related the Release/Dev test split.

@jeremylt jeremylt force-pushed the jeremy/get-fields branch 5 times, most recently from 634576b to d27cf79 Compare September 9, 2021 22:29
Comment thread rust/libceed/src/operator.rs
Comment thread interface/ceed-operator.c
@jeremylt
Copy link
Copy Markdown
Member Author

Fun new OCCA backend bug! The OCCA backend attempts to fully build and validate an Operator as part of its process for destroying an Operator, resulting in t508 failing. I'm investigating how best to fix this. There is no reason whatsoever to validate the operator fields as a prerequsite to destroying an Operator.

@jeremylt
Copy link
Copy Markdown
Member Author

Ok, the OCCA backend is pretty opaque to me and my C++ is not too great. I'll probably need help to fix this the right way. We could also just skip that test for OCCA and file another issue against the backend to be fixed sometime in the future.

Comment thread julia/LibCEED.jl/src/LibCEED.jl
@jeremylt
Copy link
Copy Markdown
Member Author

While we're here, any objections to raising these to the public API level?

CEED_EXTERN int CeedOperatorGetNumElements(CeedOperator op, CeedInt *num_elem);
CEED_EXTERN int CeedOperatorGetNumQuadraturePoints(CeedOperator op,
    CeedInt *num_qpts);

maybe also this one?

CEED_EXTERN int CeedOperatorGetQFunction(CeedOperator op, CeedQFunction *qf);

@jedbrown
Copy link
Copy Markdown
Member

While we're here, any objections to raising these to the public API level?

CEED_EXTERN int CeedOperatorGetNumElements(CeedOperator op, CeedInt *num_elem);
CEED_EXTERN int CeedOperatorGetNumQuadraturePoints(CeedOperator op,
    CeedInt *num_qpts);

maybe also this one?

CEED_EXTERN int CeedOperatorGetQFunction(CeedOperator op, CeedQFunction *qf);

Just need to clarify what happens if you call those on a composite operator. Either it consults each of its sub-operators and aggregates the result or it returns error (or something out of band, like -1).

Comment thread rust/libceed/src/qfunction.rs
@jeremylt jeremylt force-pushed the jeremy/get-fields branch 2 times, most recently from 6ebdb00 to c4ba7f8 Compare September 13, 2021 20:21
Comment thread rust/libceed/src/operator.rs
Comment thread rust/libceed/src/lib.rs Outdated
@jeremylt
Copy link
Copy Markdown
Member Author

Ok, this all should be fine to merge now - I think I'll talk a bit about our modern interfaces in my PSAAP slides

Copy link
Copy Markdown
Member

@jedbrown jedbrown left a comment

Choose a reason for hiding this comment

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

LGTM any time. We could merge now and use ref_cast later. I'd love to make progress on #807 or #805 this week.

Comment thread doc/sphinx/source/releasenotes.md Outdated
Comment on lines +138 to +144
let slice = unsafe {
std::slice::from_raw_parts(
&ptr as *const bind_ceed::CeedElemRestriction as *const crate::ElemRestriction,
1 as usize,
)
};
ElemRestrictionOpt::Some(&slice[0])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I found this safe and less contorted way to do this. https://docs.rs/ref-cast/1.0.6/ref_cast/

Co-authored-by: Jed Brown <jed@jedbrown.org>
@jeremylt jeremylt merged commit 0b54870 into main Sep 14, 2021
@jeremylt jeremylt deleted the jeremy/get-fields branch September 14, 2021 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants