Skip to content

[Java] CheckpointState AddProperty & GetProperty support #15730

Merged
baijumeswani merged 2 commits intomicrosoft:mainfrom
Craigacp:java-training-get-prop
Apr 28, 2023
Merged

[Java] CheckpointState AddProperty & GetProperty support #15730
baijumeswani merged 2 commits intomicrosoft:mainfrom
Craigacp:java-training-get-prop

Conversation

@Craigacp
Copy link
Copy Markdown
Contributor

Description

Adds the Java side methods for CheckpointState AddProperty and GetProperty, mirroring #15720. Also ports across the relevant C# tests from #15720.

Motivation and Context

Java side support for the new training APIs.

cc @baijumeswani

@baijumeswani
Copy link
Copy Markdown
Contributor

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

@baijumeswani
Copy link
Copy Markdown
Contributor

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 8 pipeline(s).

Assertions.assertEquals(7, value);

try {
String strVal = ckpt.getStringProperty(env.defaultAllocator, propertyName);

Check notice

Code scanning / CodeQL

Unread local variable

Variable 'String strVal' is never read.
}

try {
float floatVal = ckpt.getFloatProperty(env.defaultAllocator, propertyName);

Check notice

Code scanning / CodeQL

Unread local variable

Variable 'float floatVal' is never read.
Assertions.assertEquals(3.14f, value);

try {
String strVal = ckpt.getStringProperty(env.defaultAllocator, propertyName);

Check notice

Code scanning / CodeQL

Unread local variable

Variable 'String strVal' is never read.
}

try {
int intVal = ckpt.getIntProperty(env.defaultAllocator, propertyName);

Check notice

Code scanning / CodeQL

Unread local variable

Variable 'int intVal' is never read.
Assertions.assertEquals("onnxruntime", value);

try {
float floatVal = ckpt.getFloatProperty(env.defaultAllocator, propertyName);

Check notice

Code scanning / CodeQL

Unread local variable

Variable 'float floatVal' is never read.
}

try {
int intVal = ckpt.getIntProperty(env.defaultAllocator, propertyName);

Check notice

Code scanning / CodeQL

Unread local variable

Variable 'int intVal' is never read.
@Craigacp
Copy link
Copy Markdown
Contributor Author

The unused local variables are to trigger exceptions, so they need to stay. Dunno what the Windows CI failures are, they don't look related to my code.

* @return The property value.
* @throws OrtException If the property does not exist, or is of the wrong type.
*/
public int getIntProperty(String name) throws OrtException {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I kind of struggled with this myself.

I was trying to find a C# data type that resembled C++'s std::variant, but was not able to find one. I ended up using the dynamic type in the end.

We should see if this is something we can revisit later on.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In Java I can return boxed types and make a single method that returns Object, but it seemed better to have it be typesafe. The boxed types are a bit nasty to work with on the JNI side, and we needed it done quickly.

Copy link
Copy Markdown
Contributor

@baijumeswani baijumeswani 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 the prompt help with this pull request Adam. :)

@baijumeswani baijumeswani merged commit 8a1a40a into microsoft:main Apr 28, 2023
@baijumeswani baijumeswani added api:Java issues related to the Java API training issues related to ONNX Runtime training; typically submitted using template labels Apr 28, 2023
ShukantPal pushed a commit to ShukantPal/onnxruntime that referenced this pull request May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api:Java issues related to the Java API training issues related to ONNX Runtime training; typically submitted using template

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants